Fix setting max_connections in server yaml (#3619)

2026-04-23 05:13:00 -05:00 · 2022-06-16 13:00:32 -07:00
parent fbdf17d377
commit 06437b8c53
4 changed files with 191 additions and 46 deletions
@@ -153,7 +153,7 @@ func NewSqlEngine(
 	}, nil
 }

-// NewRebasedEngine returns a smalled rebased engine primarily used in filterbranch.
+// NewRebasedSqlEngine returns a smalled rebased engine primarily used in filterbranch.
 func NewRebasedSqlEngine(engine *gms.Engine, dbs map[string]dsqle.SqlDatabase) *SqlEngine {
 	return &SqlEngine{
 		dbs:    dbs,
@@ -43,11 +43,6 @@ func Serve(
 	serverController *ServerController,
 	dEnv *env.DoltEnv,
 ) (startError error, closeError error) {
-	if serverConfig == nil {
-		cli.Println("No configuration given, using defaults")
-		serverConfig = DefaultServerConfig()
-	}
-
 	// Code is easier to work through if we assume that serverController is never nil
 	if serverController == nil {
 		serverController = NewServerController()
@@ -89,17 +84,6 @@ func Serve(
 		isReadOnly = true
 	}

-	serverConf := server.Config{Protocol: "tcp"}
-
-	if serverConfig.PersistenceBehavior() == loadPerisistentGlobals {
-		serverConf, startError = serverConf.NewConfig()
-		if startError != nil {
-			return
-		}
-	}
-
-	serverConf.DisableClientMultiStatements = serverConfig.DisableClientMultiStatements()
-
 	var mrEnv *env.MultiRepoEnv
 	dbNamesAndPaths := serverConfig.DatabaseNamesAndPaths()

@@ -140,31 +124,13 @@ func Serve(
 		}
 	}

-	portAsString := strconv.Itoa(serverConfig.Port())
-	hostPort := net.JoinHostPort(serverConfig.Host(), portAsString)
-
-	if portInUse(hostPort) {
-		portInUseError := fmt.Errorf("Port %s already in use.", portAsString)
-		return portInUseError, nil
+	serverConf, sErr, cErr := getConfigFromServerConfig(serverConfig)
+	if cErr != nil {
+		return nil, cErr
+	} else if sErr != nil {
+		return sErr, nil
 	}

-	readTimeout := time.Duration(serverConfig.ReadTimeout()) * time.Millisecond
-	writeTimeout := time.Duration(serverConfig.WriteTimeout()) * time.Millisecond
-
-	tlsConfig, err := LoadTLSConfig(serverConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	// Do not set the value of Version.  Let it default to what go-mysql-server uses.  This should be equivalent
-	// to the value of mysql that we support.
-	serverConf.Address = hostPort
-	serverConf.ConnReadTimeout = readTimeout
-	serverConf.ConnWriteTimeout = writeTimeout
-	serverConf.MaxConnections = serverConfig.MaxConnections()
-	serverConf.TLSConfig = tlsConfig
-	serverConf.RequireSecureTransport = serverConfig.RequireSecureTransport()
-
 	// Create SQL Engine with users
 	config := &engine.SqlEngineConfig{
 		InitialDb:    "",
@@ -258,3 +224,50 @@ func newSessionBuilder(se *engine.SqlEngine) server.SessionBuilder {
 		return se.NewDoltSession(ctx, mysqlBaseSess)
 	}
 }
+
+// getConfigFromServerConfig processes ServerConfig and returns server.Config for sql-server.
+func getConfigFromServerConfig(serverConfig ServerConfig) (server.Config, error, error) {
+	serverConf := server.Config{Protocol: "tcp"}
+	serverConf.DisableClientMultiStatements = serverConfig.DisableClientMultiStatements()
+
+	readTimeout := time.Duration(serverConfig.ReadTimeout()) * time.Millisecond
+	writeTimeout := time.Duration(serverConfig.WriteTimeout()) * time.Millisecond
+
+	tlsConfig, err := LoadTLSConfig(serverConfig)
+	if err != nil {
+		return server.Config{}, nil, err
+	}
+
+	portAsString := strconv.Itoa(serverConfig.Port())
+	hostPort := net.JoinHostPort(serverConfig.Host(), portAsString)
+
+	if portInUse(hostPort) {
+		portInUseError := fmt.Errorf("Port %s already in use.", portAsString)
+		return server.Config{}, portInUseError, nil
+	}
+
+	// if persist is 'load' we use currently set persisted global variable,
+	// else if 'ignore' we set persisted global variable to current value from serverConfig
+	if serverConfig.PersistenceBehavior() == loadPerisistentGlobals {
+		serverConf, err = serverConf.NewConfig()
+		if err != nil {
+			return server.Config{}, err, nil
+		}
+	} else {
+		err = sql.SystemVariables.SetGlobal("max_connections", serverConfig.MaxConnections())
+		if err != nil {
+			return server.Config{}, err, nil
+		}
+	}
+
+	// Do not set the value of Version.  Let it default to what go-mysql-server uses.  This should be equivalent
+	// to the value of mysql that we support.
+	serverConf.Address = hostPort
+	serverConf.ConnReadTimeout = readTimeout
+	serverConf.ConnWriteTimeout = writeTimeout
+	serverConf.MaxConnections = serverConfig.MaxConnections()
+	serverConf.TLSConfig = tlsConfig
+	serverConf.RequireSecureTransport = serverConfig.RequireSecureTransport()
+
+	return serverConf, nil, nil
+}
@@ -21,6 +21,7 @@ import (
 	"strconv"
 	"strings"

+	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/fatih/color"

 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
@@ -140,7 +141,7 @@ func (cmd SqlServerCmd) ArgParser() *argparser.ArgParser {
 	ap.SupportsFlag(noAutoCommitFlag, "", "Set @@autocommit = off for the server")
 	ap.SupportsInt(queryParallelismFlag, "", "num-go-routines", fmt.Sprintf("Set the number of go routines spawned to handle each query (default `%d`)", serverConfig.QueryParallelism()))
 	ap.SupportsInt(maxConnectionsFlag, "", "max-connections", fmt.Sprintf("Set the number of connections handled by the server (default `%d`)", serverConfig.MaxConnections()))
-	ap.SupportsInt(persistenceBehaviorFlag, "", "persistence-behavior", fmt.Sprintf("Indicate whether to `load` or `ignore` persisted global variables (default `%s`)", serverConfig.PersistenceBehavior()))
+	ap.SupportsString(persistenceBehaviorFlag, "", "persistence-behavior", fmt.Sprintf("Indicate whether to `load` or `ignore` persisted global variables (default `%s`)", serverConfig.PersistenceBehavior()))
 	ap.SupportsString(privilegeFilePathFlag, "", "privilege file", "Path to a file to load and store users and grants. Without this flag, the database has a single user with all permissions, and more cannot be added.")
 	return ap
 }
@@ -206,6 +207,8 @@ func startServer(ctx context.Context, versionStr, commandStr string, args []stri
 	return 0
 }

+// GetServerConfig returns ServerConfig that is set either from yaml file if given, if not it is set with values defined
+// on command line. Server config variables not defined are set to default values.
 func GetServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResults) (ServerConfig, error) {
 	if cfgFile, ok := apr.GetValue(configFileFlag); ok {
 		return getYAMLServerConfig(dEnv.FS, cfgFile)
@@ -213,21 +216,31 @@ func GetServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResults) (ServerC
 	return getCommandLineServerConfig(dEnv, apr)
 }

+// getCommandLineServerConfig sets server config variables and persisted global variables with values defined on command line.
+// If not defined, it sets variables to default values.
 func getCommandLineServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResults) (ServerConfig, error) {
 	serverConfig := DefaultServerConfig()

 	if host, ok := apr.GetValue(hostFlag); ok {
 		serverConfig.withHost(host)
 	}
+
 	if port, ok := apr.GetInt(portFlag); ok {
 		serverConfig.WithPort(port)
 	}
+
 	if user, ok := apr.GetValue(userFlag); ok {
 		serverConfig.withUser(user)
 	}
+
 	if password, ok := apr.GetValue(passwordFlag); ok {
 		serverConfig.withPassword(password)
 	}
+
+	if persistenceBehavior, ok := apr.GetValue(persistenceBehaviorFlag); ok {
+		serverConfig.withPersistenceBehavior(persistenceBehavior)
+	}
+
 	if timeoutStr, ok := apr.GetValue(timeoutFlag); ok {
 		timeout, err := strconv.ParseUint(timeoutStr, 10, 64)

@@ -236,13 +249,25 @@ func getCommandLineServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResult
 		}

 		serverConfig.withTimeout(timeout * 1000)
+
+		err = sql.SystemVariables.SetGlobal("net_read_timeout", timeout*1000)
+		if err != nil {
+			return nil, fmt.Errorf("failed to set net_read_timeout. Error: %s", err.Error())
+		}
+		err = sql.SystemVariables.SetGlobal("net_write_timeout", timeout*1000)
+		if err != nil {
+			return nil, fmt.Errorf("failed to set net_write_timeout. Error: %s", err.Error())
+		}
 	}
+
 	if _, ok := apr.GetValue(readonlyFlag); ok {
 		serverConfig.withReadOnly(true)
 	}
+
 	if logLevel, ok := apr.GetValue(logLevelFlag); ok {
 		serverConfig.withLogLevel(LogLevel(logLevel))
 	}
+
 	if multiDBDir, ok := apr.GetValue(multiDBDirFlag); ok {
 		dbNamesAndPaths, err := env.DBNamesAndPathsFromDir(dEnv.FS, multiDBDir)

@@ -261,13 +286,13 @@ func getCommandLineServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResult

 	if maxConnections, ok := apr.GetInt(maxConnectionsFlag); ok {
 		serverConfig.withMaxConnections(uint64(maxConnections))
+		err := sql.SystemVariables.SetGlobal("max_connections", uint64(maxConnections))
+		if err != nil {
+			return nil, fmt.Errorf("failed to set max_connections. Error: %s", err.Error())
+		}
 	}

 	serverConfig.autoCommit = !apr.Contains(noAutoCommitFlag)
-	if persistenceBehavior, ok := apr.GetValue(persistenceBehaviorFlag); ok {
-		serverConfig.withPersistenceBehavior(persistenceBehavior)
-	}
-
 	if privilegeFilePath, ok := apr.GetValue(privilegeFilePathFlag); ok {
 		serverConfig.withPrivilegeFilePath(privilegeFilePath)
 	}
@@ -275,6 +300,7 @@ func getCommandLineServerConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResult
 	return serverConfig, nil
 }

+// getYAMLServerConfig returns server config variables with values defined in yaml file.
 func getYAMLServerConfig(fs filesys.Filesys, path string) (ServerConfig, error) {
 	data, err := fs.ReadFile(path)
 	if err != nil {
@@ -286,5 +312,24 @@ func getYAMLServerConfig(fs filesys.Filesys, path string) (ServerConfig, error)
 		return nil, fmt.Errorf("Failed to parse yaml file '%s'. Error: %s", path, err.Error())
 	}

+	if cfg.ListenerConfig.MaxConnections != nil {
+		err = sql.SystemVariables.SetGlobal("max_connections", *cfg.ListenerConfig.MaxConnections)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to set max_connections from yaml file '%s'. Error: %s", path, err.Error())
+		}
+	}
+	if cfg.ListenerConfig.ReadTimeoutMillis != nil {
+		err = sql.SystemVariables.SetGlobal("net_read_timeout", *cfg.ListenerConfig.ReadTimeoutMillis)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to set net_read_timeout from yaml file '%s'. Error: %s", path, err.Error())
+		}
+	}
+	if cfg.ListenerConfig.WriteTimeoutMillis != nil {
+		err = sql.SystemVariables.SetGlobal("net_write_timeout", *cfg.ListenerConfig.WriteTimeoutMillis)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to set net_write_timeout from yaml file '%s'. Error: %s", path, err.Error())
+		}
+	}
+
 	return cfg, nil
 }
@@ -27,7 +27,6 @@ teardown() {
    start_sql_server repo1

    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n1000"
-
 }

@test "sql-server-config: invalid persisted global variable name throws warning on server startup, but does not crash" {
@@ -153,3 +152,91 @@ teardown() {
    [[ ! "$output" =~ "sqlserver.global.max_connections = 1000" ]] || false
    [[ ! "$output" =~ "sqlserver.global.auto_increment_increment = 1000" ]] || false
 }
+
+@test "sql-server-config: set max_connections with yaml config" {
+    cd repo1
+    DEFAULT_DB="repo1"
+    let PORT="$$ % (65536-1024) + 1024"
+    echo "
+log_level: debug
+
+user:
+  name: dolt
+
+listener:
+  host: 0.0.0.0
+  port: $PORT
+  max_connections: 999
+
+behavior:
+  read_only: false
+  autocommit: true" > server.yaml
+    dolt sql-server --config server.yaml &
+    SERVER_PID=$!
+    wait_for_connection $PORT 5000
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n999"
+}
+
+@test "sql-server-config: set max_connections with yaml config with persistence ignore" {
+    cd repo1
+    DEFAULT_DB="repo1"
+    let PORT="$$ % (65536-1024) + 1024"
+    echo "
+log_level: debug
+
+user:
+  name: dolt
+
+listener:
+  host: 0.0.0.0
+  port: $PORT
+  max_connections: 999
+
+behavior:
+  read_only: false
+  autocommit: true
+  persistence_behavior: ignore" > server.yaml
+
+    dolt sql-server --config server.yaml --max-connections 333 &
+    SERVER_PID=$!
+    wait_for_connection $PORT 5000
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n999"
+}
+
+@test "sql-server-config: persistence behavior set to load" {
+    cd repo1
+    start_sql_server_with_args --host 0.0.0.0 --user dolt --persistence-behavior load repo1
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n151"
+}
+
+@test "sql-server-config: persistence behavior set to ignore" {
+    cd repo1
+    start_sql_server_with_args --host 0.0.0.0 --user dolt --persistence-behavior ignore repo1
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n100"
+}
+
+@test "sql-server-config: persisted global variable defined on the command line" {
+    cd repo1
+    start_sql_server_with_args --host 0.0.0.0 --user dolt --max-connections 555 repo1
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n555"
+}
+
+@test "sql-server-config: persist global variable before server startup with persistence behavior with ignore" {
+    cd repo1
+    echo '{"sqlserver.global.max_connections":"1000"}' > .dolt/config.json
+    start_sql_server_with_args --host 0.0.0.0 --user dolt --persistence-behavior ignore repo1
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n100"
+}
+
+@test "sql-server-config: persisted global variable defined on the command line with persistence ignored" {
+    cd repo1
+    start_sql_server_with_args --host 0.0.0.0 --user dolt --max-connections 555 --persistence-behavior ignore repo1
+
+    server_query repo1 1 "select @@GLOBAL.max_connections" "@@GLOBAL.max_connections\n555"
+}