Add fallback cluster config option

doc/http.md

@@ -98,9 +98,9 @@ Notes:
 
   However this check is known to be useful, at least in one common scenario: a monitoring of a MySQL cluster based on replication lag. In such case, we may have write requests followed by read requests. We may happen to know the elapsed time between write & read. As an example, say `2.5s` have passed between the write and read. The check `/check-read/archive/mysql/main1/2.5` confirms or denies that relevant replicas are up-to-date for the `2.5s` elapsed time. We can therefore read from the replicas and safely expect to find the data we wrote `2.5s` ago on the master.
 
-- `/check-if-exists/<app>/<store-type>/<store-name>`: like `/check`, but if the metric is unknown (e.g. `<store-name>` not in `freno`'s configuration), return `200 OK`. This is useful for hybrid systems where some metrics need to be strictly controlled, and some not. `freno` would probe the important stores, and still can serve requests for all stores.
+- `/check-if-exists/<app>/<store-type>/<store-name>`: like `/check`, but if the metric is unknown (e.g. `<store-name>` not in `freno`'s configuration), return `200 OK`. Unknown names do not use MySQL's configured `FallbackCluster`. This is useful for hybrid systems where some metrics need to be strictly controlled, and some not. `freno` would probe the important stores, and still can serve requests for all stores.
 
-- `/check-read-if-exists/<app>/<store-type>/<store-name>/<threshold>`: like `/check-read`, but if the metric is unknown (e.g. `<store-name>` not in `freno`'s configuration), return `200 OK`. This is useful for hybrid systems where some metrics need to be strictly controlled, and some not. `freno` would probe the important stores, and still can serve requests for all stores.
+- `/check-read-if-exists/<app>/<store-type>/<store-name>/<threshold>`: like `/check-read`, but if the metric is unknown (e.g. `<store-name>` not in `freno`'s configuration), return `200 OK`. Unknown names do not use MySQL's configured `FallbackCluster`. This is useful for hybrid systems where some metrics need to be strictly controlled, and some not. `freno` would probe the important stores, and still can serve requests for all stores.
 
 - `/skip-host/<hostname>/ttl/<ttl-minutes>`: skip host when aggregating metrics for specified number of minutes. If host is already skipped, update the TTL.
 - `/skip-host/<hostname>`: same as `/skip-host/<hostname>/ttl/60`

doc/mysql.md

@@ -45,6 +45,7 @@ You will find the top-level configuration:
     "us-east-1",
     "us-east-2"
   ],
+  "FallbackCluster": "",
   "Clusters": {
   }
 }
@@ -75,6 +76,7 @@ These params apply in general to all MySQL clusters, unless specified differentl
   You may override `HttpCheckPath` on specific clusters.
 - `IgnoreHosts`: array of substrings. A host is completely ignored by `freno` if it contains a substring listed in `IgnoreHosts`.
   Like other values, this value can be overridden per-cluster. A non-empty `IgnoreHosts` in a specific cluster will replace the `MySQL` scope definition, for that cluster. An empty `IgnoreHosts` in a cluster scope will not un-ignore the patterns specified in `MySQL` scope. If you want to un-ignore the `MySQL` scope use some thing like `"IgnoreHosts": ["--no-such-pattern--"],`, known to never match any of your hosts.
+- `FallbackCluster`: optional configured cluster used by `/check` and `/check-read` when the requested MySQL cluster name is not configured. The value must exactly match a key in `Clusters`. Exact configured names always take precedence, including when their metric or threshold is unavailable.
 
 Looking at clusters configuration:
 
@@ -112,7 +114,7 @@ Looking at clusters configuration:
 }
 ```
 
-This introduces two clusters: `prod4` and `local`. `freno` will only serve requests for these two clusters. Any other request (e.g. `/check/archive/mysql/prod7`) will be answered with `HTTP 500` -- an unknown metric
+This introduces the `prod4`, `sharded`, and `local` clusters. Without `FallbackCluster`, `freno` only serves requests for configured clusters; any other request (e.g. `/check/archive/mysql/prod7`) is answered with `HTTP 404`. With `"FallbackCluster": "prod4"`, ordinary `/check` and `/check-read` requests for unknown MySQL cluster names use `prod4`'s metric, threshold, and throttling state.
 
 Noteworthy:
 

pkg/config/config_test.go

@@ -85,6 +85,33 @@ func TestReload(t *testing.T) {
 	}
 }
 
+func TestMySQLFallbackCluster(t *testing.T) {
+	tests := []struct {
+		name            string
+		fallbackCluster string
+		clusters        map[string]*MySQLClusterConfigurationSettings
+		wantErr         string
+	}{
+		{name: "omitted"},
+		{name: "empty", clusters: map[string]*MySQLClusterConfigurationSettings{"primary": {}}},
+		{name: "configured", fallbackCluster: "primary", clusters: map[string]*MySQLClusterConfigurationSettings{"primary": {}}},
+		{name: "missing", fallbackCluster: "missing", clusters: map[string]*MySQLClusterConfigurationSettings{"primary": {}}, wantErr: `Stores.MySQL.FallbackCluster "missing" does not name a configured cluster`},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			settings := MySQLConfigurationSettings{FallbackCluster: test.fallbackCluster, Clusters: test.clusters}
+			err := settings.postReadAdjustments()
+			if test.wantErr == "" && err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if test.wantErr != "" && (err == nil || err.Error() != test.wantErr) {
+				t.Fatalf("error = %v, want %q", err, test.wantErr)
+			}
+		})
+	}
+}
+
 func dump(path string, contents *ConfigurationSettings) error {
 	json, _ := json.Marshal(contents)
 	err := ioutil.WriteFile(path, json, 0644)

pkg/config/mysql_config.go

@@ -5,6 +5,7 @@ package config
 //
 
 import (
+	"fmt"
 	"os"
 )
 
@@ -65,14 +66,20 @@ type MySQLConfigurationSettings struct {
 	VitessCells          []string // Name of the Vitess cells for polling tablet hosts
 	Collation            string   // MySQL collation to use for stores, replaces charset if specified
 
-	Clusters map[string](*MySQLClusterConfigurationSettings) // cluster name -> cluster config
+	FallbackCluster string
+	Clusters        map[string](*MySQLClusterConfigurationSettings) // cluster name -> cluster config
 }
 
 // Hook to implement adjustments after reading each configuration file.
 func (settings *MySQLConfigurationSettings) postReadAdjustments() error {
 	if settings.Port == 0 {
 		settings.Port = DefaultMySQLPort
 	}
+	if settings.FallbackCluster != "" {
+		if _, ok := settings.Clusters[settings.FallbackCluster]; !ok {
+			return fmt.Errorf("Stores.MySQL.FallbackCluster %q does not name a configured cluster", settings.FallbackCluster)
+		}
+	}
 	// Username & password may be given as plaintext in the config file, or can be delivered
 	// via environment variables. We accept user & password in the form "${SOME_ENV_VARIABLE}"
 	// in which case we get the value from this process' invoking environment.

pkg/http/api.go

@@ -49,7 +49,6 @@ type API interface {
 
 var endpoints = []string{} // known API URIs
 
-var okIfNotExistsFlags = &throttle.CheckFlags{OKIfNotExists: true}
 var metricsHandler = exp.ExpHandler(metrics.DefaultRegistry)
 
 type GeneralResponse struct {
@@ -167,11 +166,14 @@ func (api *APIImpl) check(w http.ResponseWriter, r *http.Request, ps httprouter.
 		remoteAddr = r.RemoteAddr
 		remoteAddr = strings.Split(remoteAddr, ":")[0]
 	}
-	flags.LowPriority = (r.URL.Query().Get("p") == "low")
-
-	checkResult := api.throttlerCheck.Check(appName, storeType, storeName, remoteAddr, flags)
-	if checkResult.StatusCode == http.StatusNotFound && flags.OKIfNotExists {
-		checkResult.StatusCode = http.StatusOK // 200
+	requestFlags := *flags
+	requestFlags.LowPriority = (r.URL.Query().Get("p") == "low")
+
+	checkResult := api.throttlerCheck.Check(appName, storeType, storeName, remoteAddr, &requestFlags)
+	if checkResult.StatusCode == http.StatusNotFound && requestFlags.OKIfNotExists {
+		result := *checkResult
+		result.StatusCode = http.StatusOK // 200
+		checkResult = &result
 	}
 
 	api.respondToCheckRequest(w, r, checkResult)
@@ -185,7 +187,7 @@ func (api *APIImpl) WriteCheck(w http.ResponseWriter, r *http.Request, ps httpro
 // WriteCheckIfExists checks for a metric, but reports an OK if the metric does not exist.
 // If the metric does exist, then all usual checks are made.
 func (api *APIImpl) WriteCheckIfExists(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	api.check(w, r, ps, okIfNotExistsFlags)
+	api.check(w, r, ps, &throttle.CheckFlags{OKIfNotExists: true})
 }
 
 func (api *APIImpl) readCheck(w http.ResponseWriter, r *http.Request, ps httprouter.Params, flags *throttle.CheckFlags) {

pkg/throttle/api_fallback_test.go

@@ -0,0 +1,45 @@
+package throttle_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/github/freno/pkg/config"
+	frenohttp "github.com/github/freno/pkg/http"
+	"github.com/github/freno/pkg/throttle"
+)
+
+func TestFallbackHTTPCheckEndpoints(t *testing.T) {
+	defer config.Reset()
+	config.Settings().Stores.MySQL = config.MySQLConfigurationSettings{
+		FallbackCluster: "fallback",
+		Clusters: map[string]*config.MySQLClusterConfigurationSettings{
+			"fallback": {},
+		},
+	}
+
+	throttler := throttle.NewThrottler()
+	throttle.SetTestMySQLClusterMetric(throttler, "fallback", 10.0, 5.0)
+	router := frenohttp.ConfigureRoutes(frenohttp.NewAPIImpl(throttle.NewThrottlerCheck(throttler), nil))
+
+	tests := []struct {
+		path string
+		want int
+	}{
+		{path: "/check/test-app/mysql/unknown", want: http.StatusTooManyRequests},
+		{path: "/check-read/test-app/mysql/unknown/20", want: http.StatusOK},
+		{path: "/check-if-exists/test-app/mysql/unknown", want: http.StatusOK},
+		{path: "/check-read-if-exists/test-app/mysql/unknown/5", want: http.StatusOK},
+	}
+
+	for _, test := range tests {
+		t.Run(test.path, func(t *testing.T) {
+			recorder := httptest.NewRecorder()
+			router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, test.path, nil))
+			if recorder.Code != test.want {
+				t.Fatalf("status = %d, want %d; body: %s", recorder.Code, test.want, recorder.Body.String())
+			}
+		})
+	}
+}

pkg/throttle/check.go

@@ -1,15 +1,14 @@
 package throttle
 
 import (
+	"errors"
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
 
-	"fmt"
-
-	"errors"
-
 	"github.com/github/freno/pkg/base"
+	"github.com/github/freno/pkg/config"
 	metrics "github.com/rcrowley/go-metrics"
 )
 
@@ -101,12 +100,27 @@ func (check *ThrottlerCheck) checkAppMetricResult(appName string, storeType stri
 
 // CheckAppStoreMetric
 func (check *ThrottlerCheck) Check(appName string, storeType string, storeName string, remoteAddr string, flags *CheckFlags) (checkResult *CheckResult) {
+	requestedStoreName := storeName
 	var metricResultFunc base.MetricResultFunc
 	switch storeType {
 	case "mysql":
 		{
-			metricResultFunc = func() (metricResult base.MetricResult, threshold float64) {
-				return check.throttler.getMySQLClusterMetrics(storeName)
+			mysqlSettings := config.Settings().Stores.MySQL
+			_, configured := mysqlSettings.Clusters[storeName]
+			if !configured && !flags.OKIfNotExists {
+				if fallbackCluster := mysqlSettings.FallbackCluster; fallbackCluster != "" {
+					storeName = fallbackCluster
+					_, configured = mysqlSettings.Clusters[storeName]
+				}
+			}
+			if configured {
+				metricResultFunc = func() (metricResult base.MetricResult, threshold float64) {
+					return check.throttler.getMySQLClusterMetrics(storeName)
+				}
+			} else {
+				metricResultFunc = func() (metricResult base.MetricResult, threshold float64) {
+					return base.NoSuchMetric, 0
+				}
 			}
 		}
 	}
@@ -120,22 +134,22 @@ func (check *ThrottlerCheck) Check(appName string, storeType string, storeName s
 		metrics.GetOrRegisterCounter("check.any.total", nil).Inc(1)
 		metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.total", appName), nil).Inc(1)
 
-		metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.total", storeType, storeName), nil).Inc(1)
-		metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.total", appName, storeType, storeName), nil).Inc(1)
+		metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.total", storeType, requestedStoreName), nil).Inc(1)
+		metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.total", appName, storeType, requestedStoreName), nil).Inc(1)
 
 		if statusCode != http.StatusOK {
 			metrics.GetOrRegisterCounter("check.any.error", nil).Inc(1)
 			metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.error", appName), nil).Inc(1)
 
-			metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.error", storeType, storeName), nil).Inc(1)
-			metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.error", appName, storeType, storeName), nil).Inc(1)
+			metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.error", storeType, requestedStoreName), nil).Inc(1)
+			metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.error", appName, storeType, requestedStoreName), nil).Inc(1)
 
 			if statusCode == http.StatusInternalServerError {
 				metrics.GetOrRegisterCounter("check.any.internal-error", nil).Inc(1)
 				metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.internal-error", appName), nil).Inc(1)
 
-				metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.internal-error", storeType, storeName), nil).Inc(1)
-				metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.internal-error", appName, storeType, storeName), nil).Inc(1)
+				metrics.GetOrRegisterCounter(fmt.Sprintf("check.any.%s.%s.internal-error", storeType, requestedStoreName), nil).Inc(1)
+				metrics.GetOrRegisterCounter(fmt.Sprintf("check.%s.%s.%s.internal-error", appName, storeType, requestedStoreName), nil).Inc(1)
 			}
 		}