Merge pull request #155 from github/add-collation-flag

Add collation settings to allow collation to override charset

.github/workflows/main.yml

@@ -13,7 +13,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v1
       with:
-        go-version: 1.17
+        go-version: 1.19
       id: go
 
     - name: Build

.github/workflows/release.yml

@@ -18,7 +18,7 @@ jobs:
         name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: 1.17
+          go-version: 1.19
       -
         name: Run GoReleaser
         uses: goreleaser/goreleaser-action@v2

internal/raft/net_transport.go

@@ -37,7 +37,6 @@ var (
 )
 
 /*
-
 NetworkTransport provides a network based transport that can be
 used to communicate with Raft on remote machines. It requires
 an underlying stream layer to provide a stream abstraction, which can
@@ -53,7 +52,6 @@ both are encoded using MsgPack.
 InstallSnapshot is special, in that after the RPC request we stream
 the entire state. That socket is not re-used as the connection state
 is not known if there is an error.
-
 */
 type NetworkTransport struct {
 	connPool     map[string][]*netConn

pkg/config/config.go

@@ -92,24 +92,25 @@ func (config *Configuration) Reload() error {
 // Some of the settinges have reasonable default values, and some other
 // (like database credentials) are strictly expected from user.
 type ConfigurationSettings struct {
-	ListenPort           int
-	DataCenter           string
-	Environment          string
-	Domain               string
-	ShareDomain          string
-	RaftBind             string
-	RaftDataDir          string
-	DefaultRaftPort      int      // if a RaftNodes entry does not specify port, use this one
-	RaftNodes            []string // Raft nodes to make initial connection with
-	BackendMySQLHost     string
-	BackendMySQLPort     int
-	BackendMySQLSchema   string
-	BackendMySQLUser     string
-	BackendMySQLPassword string
-	MemcacheServers      []string // if given, freno will report to aggregated values to given memcache
-	MemcachePath         string   // use as prefix to metric path in memcache key, e.g. if `MemcachePath` is "myprefix" the key would be "myprefix/mysql/maincluster". Default: "freno"
-	EnableProfiling      bool     // enable pprof profiling http api
-	Stores               StoresSettings
+	ListenPort            int
+	DataCenter            string
+	Environment           string
+	Domain                string
+	ShareDomain           string
+	RaftBind              string
+	RaftDataDir           string
+	DefaultRaftPort       int      // if a RaftNodes entry does not specify port, use this one
+	RaftNodes             []string // Raft nodes to make initial connection with
+	BackendMySQLHost      string
+	BackendMySQLPort      int
+	BackendMySQLSchema    string
+	BackendMySQLUser      string
+	BackendMySQLPassword  string
+	BackendMySQLCollation string   // if specified, use this collation instead of charset when connecting to MySQL backend
+	MemcacheServers       []string // if given, freno will report to aggregated values to given memcache
+	MemcachePath          string   // use as prefix to metric path in memcache key, e.g. if `MemcachePath` is "myprefix" the key would be "myprefix/mysql/maincluster". Default: "freno"
+	EnableProfiling       bool     // enable pprof profiling http api
+	Stores                StoresSettings
 }
 
 func newConfigurationSettings() *ConfigurationSettings {

pkg/config/mysql_config.go

@@ -63,6 +63,7 @@ type MySQLConfigurationSettings struct {
 	ProxySQLUser         string   // ProxySQL stats username
 	ProxySQLPassword     string   // ProxySQL stats password
 	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
 }

pkg/group/mysql.go

@@ -77,9 +77,7 @@ func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) {
 	if config.Settings().BackendMySQLHost == "" {
 		return nil, nil
 	}
-	dbUri := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=500ms",
-		config.Settings().BackendMySQLUser, config.Settings().BackendMySQLPassword, config.Settings().BackendMySQLHost, config.Settings().BackendMySQLPort, config.Settings().BackendMySQLSchema,
-	)
+	dbUri := getBackendDBUri()
 	db, _, err := sqlutils.GetDB(dbUri)
 	if err != nil {
 		return nil, err
@@ -108,6 +106,23 @@ func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) {
 	return backend, nil
 }
 
+// helper function to get the DB URI
+func getBackendDBUri() string {
+	dsnCharsetCollation := "charset=utf8mb4,utf8,latin1"
+	if config.Settings().BackendMySQLCollation != "" {
+		// Set collation instead of charset, if BackendMySQLCollation is specified
+		dsnCharsetCollation = fmt.Sprintf("collation=%s", config.Settings().BackendMySQLCollation)
+	}
+	return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&%s&timeout=500ms",
+		config.Settings().BackendMySQLUser,
+		config.Settings().BackendMySQLPassword,
+		config.Settings().BackendMySQLHost,
+		config.Settings().BackendMySQLPort,
+		config.Settings().BackendMySQLSchema,
+		dsnCharsetCollation,
+	)
+}
+
 // Monitor is a utility function to routinely observe leadership state.
 // It doesn't actually do much; merely takes notes.
 func (backend *MySQLBackend) continuousOperations() {

pkg/group/mysql_test.go

@@ -0,0 +1,35 @@
+/*
+   Copyright 2023 GitHub Inc.
+	 See https://github.com/github/freno/blob/master/LICENSE
+*/
+
+package group
+
+import (
+	"testing"
+
+	"github.com/github/freno/pkg/config"
+	"github.com/outbrain/golib/log"
+	test "github.com/outbrain/golib/tests"
+)
+
+func init() {
+	log.SetLevel(log.ERROR)
+}
+
+func TestGetBackendDBUri(t *testing.T) {
+	config.Settings().BackendMySQLUser = "gromit"
+	config.Settings().BackendMySQLPassword = "penguin"
+	config.Settings().BackendMySQLHost = "myhost"
+	config.Settings().BackendMySQLPort = 3306
+	config.Settings().BackendMySQLSchema = "test_database"
+
+	// test default (charset)
+	dbUri := getBackendDBUri()
+	test.S(t).ExpectEquals(dbUri, "gromit:penguin@tcp(myhost:3306)/test_database?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=500ms")
+
+	// test setting collation
+	config.Settings().BackendMySQLCollation = "utf8mb4_unicode_ci"
+	dbUri = getBackendDBUri()
+	test.S(t).ExpectEquals(dbUri, "gromit:penguin@tcp(myhost:3306)/test_database?interpolateParams=true&collation=utf8mb4_unicode_ci&timeout=500ms")
+}

pkg/mysql/probe.go

@@ -8,6 +8,8 @@ package mysql
 import (
 	"fmt"
 	"net"
+
+	"github.com/github/freno/pkg/config"
 )
 
 const maxPoolConnections = 3
@@ -48,33 +50,46 @@ func NewProbe() *Probe {
 }
 
 // DuplicateCredentials creates a new connection config with given key and with same credentials as this config
-func (this *Probe) DuplicateCredentials(key InstanceKey) *Probe {
+func (probe *Probe) DuplicateCredentials(key InstanceKey) *Probe {
 	config := &Probe{
 		Key:      key,
-		User:     this.User,
-		Password: this.Password,
+		User:     probe.User,
+		Password: probe.Password,
 	}
 	return config
 }
 
-func (this *Probe) Duplicate() *Probe {
-	return this.DuplicateCredentials(this.Key)
+func (probe *Probe) Duplicate() *Probe {
+	return probe.DuplicateCredentials(probe.Key)
 }
 
-func (this *Probe) String() string {
-	return fmt.Sprintf("%s, user=%s", this.Key.DisplayString(), this.User)
+func (probe *Probe) String() string {
+	return fmt.Sprintf("%s, user=%s", probe.Key.DisplayString(), probe.User)
 }
 
-func (this *Probe) Equals(other *Probe) bool {
-	return this.Key.Equals(&other.Key)
+func (probe *Probe) Equals(other *Probe) bool {
+	return probe.Key.Equals(&other.Key)
 }
 
-func (this *Probe) GetDBUri(databaseName string) string {
-	hostname := this.Key.Hostname
+func (probe *Probe) GetDBUri(databaseName string) string {
+	hostname := probe.Key.Hostname
 	var ip = net.ParseIP(hostname)
 	if (ip != nil) && (ip.To4() == nil) {
 		// Wrap IPv6 literals in square brackets
 		hostname = fmt.Sprintf("[%s]", hostname)
 	}
-	return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=%dms", this.User, this.Password, hostname, this.Key.Port, databaseName, timeoutMillis)
+	dsnCharsetCollation := "charset=utf8mb4,utf8,latin1"
+	if config.Settings().Stores.MySQL.Collation != "" {
+		// Set collation instead of charset, if Stores.MySQL.Collation is specified
+		dsnCharsetCollation = fmt.Sprintf("collation=%s", config.Settings().Stores.MySQL.Collation)
+	}
+	return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&%s&timeout=%dms",
+		probe.User,
+		probe.Password,
+		hostname,
+		probe.Key.Port,
+		databaseName,
+		dsnCharsetCollation,
+		timeoutMillis,
+	)
 }

pkg/mysql/probe_test.go

@@ -8,6 +8,7 @@ package mysql
 import (
 	"testing"
 
+	"github.com/github/freno/pkg/config"
 	"github.com/outbrain/golib/log"
 	test "github.com/outbrain/golib/tests"
 )
@@ -49,3 +50,19 @@ func TestDuplicate(t *testing.T) {
 	test.S(t).ExpectEquals(dup.User, "gromit")
 	test.S(t).ExpectEquals(dup.Password, "penguin")
 }
+
+func TestGetDBUri(t *testing.T) {
+	c := NewProbe()
+	c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
+	c.User = "gromit"
+	c.Password = "penguin"
+
+	// test default (charset)
+	dbUri := c.GetDBUri("test_database")
+	test.S(t).ExpectEquals(dbUri, "gromit:penguin@tcp(myhost:3306)/test_database?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=1000ms")
+
+	// test setting collation
+	config.Settings().Stores.MySQL.Collation = "utf8mb4_unicode_ci"
+	dbUri = c.GetDBUri("test_database")
+	test.S(t).ExpectEquals(dbUri, "gromit:penguin@tcp(myhost:3306)/test_database?interpolateParams=true&collation=utf8mb4_unicode_ci&timeout=1000ms")
+}

pkg/proxysql/client_test.go

@@ -51,8 +51,8 @@ func TestProxySQLGetDB(t *testing.T) {
 		if err == nil {
 			t.Fatal("expected error for failed connection")
 		}
-		if !strings.HasSuffix(err.Error(), "no such host") {
-			t.Fatalf("expected a 'no such host' error, got %v", err)
+		if !strings.HasSuffix(err.Error(), "no such host") && !strings.HasSuffix(err.Error(), "i/o timeout") {
+			t.Fatalf("expected a 'no such host' or 'i/o timeout' error, got %v", err)
 		}
 	})
 }

script/bootstrap

@@ -2,7 +2,7 @@
 
 set -e
 
-readonly supportedGo="go1.1[67]"
+readonly supportedGo="go1.1[6789]"
 
 # Ensure go is installed
 if ! command -v go ; then