Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed persitence behavior from config #8645

Merged
merged 5 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions go/cmd/dolt/commands/sqlserver/command_line_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ type commandLineServerConfig struct {
requireSecureTransport bool
maxLoggedQueryLen int
shouldEncodeLoggedQuery bool
persistenceBehavior string
privilegeFilePath string
branchControlFilePath string
allowCleartextPasswords bool
Expand All @@ -73,7 +72,6 @@ func DefaultCommandLineServerConfig() *commandLineServerConfig {
autoCommit: servercfg.DefaultAutoCommit,
maxConnections: servercfg.DefaultMaxConnections,
queryParallelism: servercfg.DefaultQueryParallelism,
persistenceBehavior: servercfg.DefaultPersistenceBahavior,
dataDir: servercfg.DefaultDataDir,
cfgDir: filepath.Join(servercfg.DefaultDataDir, servercfg.DefaultCfgDir),
privilegeFilePath: filepath.Join(servercfg.DefaultDataDir, servercfg.DefaultCfgDir, servercfg.DefaultPrivilegeFilePath),
Expand Down Expand Up @@ -124,10 +122,6 @@ func NewCommandLineConfig(creds *cli.UserPassword, apr *argparser.ArgParseResult
config.WithRemotesapiReadOnly(&val)
}

if persistenceBehavior, ok := apr.GetValue(persistenceBehaviorFlag); ok {
config.withPersistenceBehavior(persistenceBehavior)
}

if timeoutStr, ok := apr.GetValue(timeoutFlag); ok {
timeout, err := strconv.ParseUint(timeoutStr, 10, 64)

Expand Down Expand Up @@ -241,11 +235,6 @@ func (cfg *commandLineServerConfig) QueryParallelism() int {
return cfg.queryParallelism
}

// PersistenceBehavior returns whether to autoload persisted server configuration
func (cfg *commandLineServerConfig) PersistenceBehavior() string {
return cfg.persistenceBehavior
}

// TLSKey returns a path to the servers PEM-encoded private TLS key. "" if there is none.
func (cfg *commandLineServerConfig) TLSKey() string {
return cfg.tlsKey
Expand Down Expand Up @@ -420,12 +409,6 @@ func (cfg *commandLineServerConfig) withCfgDir(cfgDir string) *commandLineServer
return cfg
}

// withPersistenceBehavior updates persistence behavior of system globals on server init
func (cfg *commandLineServerConfig) withPersistenceBehavior(persistenceBehavior string) *commandLineServerConfig {
cfg.persistenceBehavior = persistenceBehavior
return cfg
}

// withPrivilegeFilePath updates the path to the file which contains all needed privilege information in the form of a JSON string
func (cfg *commandLineServerConfig) withPrivilegeFilePath(privFilePath string) *commandLineServerConfig {
cfg.privilegeFilePath = privFilePath
Expand Down
15 changes: 3 additions & 12 deletions go/cmd/dolt/commands/sqlserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,18 +956,9 @@ func getConfigFromServerConfig(serverConfig servercfg.ServerConfig) (server.Conf
return server.Config{}, err
}

// 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() == servercfg.LoadPerisistentGlobals {
serverConf, err = serverConf.NewConfig()
if err != nil {
return server.Config{}, err
}
} else {
err = sql.SystemVariables.SetGlobal("max_connections", serverConfig.MaxConnections())
if err != nil {
return server.Config{}, err
}
serverConf, err = serverConf.NewConfig()
if err != nil {
return server.Config{}, err
}

// Do not set the value of Version. Let it default to what go-mysql-server uses. This should be equivalent
Expand Down
2 changes: 0 additions & 2 deletions go/cmd/dolt/commands/sqlserver/sqlserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const (
configFileFlag = "config"
queryParallelismFlag = "query-parallelism"
maxConnectionsFlag = "max-connections"
persistenceBehaviorFlag = "persistence-behavior"
allowCleartextPasswordsFlag = "allow-cleartext-passwords"
socketFlag = "socket"
remotesapiPortFlag = "remotesapi-port"
Expand Down Expand Up @@ -172,7 +171,6 @@ func (cmd SqlServerCmd) ArgParserWithName(name string) *argparser.ArgParser {
ap.SupportsFlag(noAutoCommitFlag, "", "Set @@autocommit = off for the server.")
ap.SupportsInt(queryParallelismFlag, "", "num-go-routines", "Deprecated, no effect in current versions of Dolt")
ap.SupportsInt(maxConnectionsFlag, "", "max-connections", fmt.Sprintf("Set the number of connections handled by the server. Defaults to `%d`.", serverConfig.MaxConnections()))
ap.SupportsString(persistenceBehaviorFlag, "", "persistence-behavior", fmt.Sprintf("Indicate whether to `load` or `ignore` persisted global variables. Defaults to `%s`.", serverConfig.PersistenceBehavior()))
ap.SupportsString(commands.PrivsFilePathFlag, "", "privilege file", "Path to a file to load and store users and grants. Defaults to `$doltcfg-dir/privileges.db`. Will be created as needed.")
ap.SupportsString(commands.BranchCtrlPathFlag, "", "branch control file", "Path to a file to load and store branch control permissions. Defaults to `$doltcfg-dir/branch_control.db`. Will be created as needed.")
ap.SupportsString(allowCleartextPasswordsFlag, "", "allow-cleartext-passwords", "Allows use of cleartext passwords. Defaults to false.")
Expand Down
9 changes: 0 additions & 9 deletions go/libraries/doltcore/servercfg/serverconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const (
DefaultDoltTransactionCommit = false
DefaultMaxConnections = 100
DefaultQueryParallelism = 0
DefaultPersistenceBahavior = LoadPerisistentGlobals
DefaultDataDir = "."
DefaultCfgDir = ".doltcfg"
DefaultPrivilegeFilePath = "privileges.db"
Expand All @@ -64,11 +63,6 @@ const (
DefaultEncodeLoggedQuery = false
)

const (
IgnorePeristentGlobals = "ignore"
LoadPerisistentGlobals = "load"
)

func ptr[T any](t T) *T {
return &t
}
Expand Down Expand Up @@ -169,8 +163,6 @@ type ServerConfig interface {
// If true, queries will be logged as base64 encoded strings.
// If false (default behavior), queries will be logged as strings, but newlines and tabs will be replaced with spaces.
ShouldEncodeLoggedQuery() bool
// PersistenceBehavior is "load" if we include persisted system globals on server init
PersistenceBehavior() string
// DisableClientMultiStatements is true if we want the server to not
// process incoming ComQuery packets as if they had multiple queries in
// them, even if the client advertises support for MULTI_STATEMENTS.
Expand Down Expand Up @@ -218,7 +210,6 @@ func DefaultServerConfig() ServerConfig {
BehaviorConfig: BehaviorYAMLConfig{
ReadOnly: ptr(DefaultReadOnly),
AutoCommit: ptr(DefaultAutoCommit),
PersistenceBehavior: ptr(DefaultPersistenceBahavior),
DoltTransactionCommit: ptr(DefaultDoltTransactionCommit),
},
UserConfig: UserYAMLConfig{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ EncodeLoggedQuery *bool 0.0.0 encode_logged_query,omitempty
BehaviorConfig servercfg.BehaviorYAMLConfig 0.0.0 behavior
-ReadOnly *bool 0.0.0 read_only
-AutoCommit *bool 0.0.0 autocommit
-PersistenceBehavior *string 0.0.0 persistence_behavior
-PersistenceBehavior *string 0.0.0 persistence_behavior,omitempty
-DisableClientMultiStatements *bool 0.0.0 disable_client_multi_statements
-DoltTransactionCommit *bool 0.0.0 dolt_transaction_commit
-EventSchedulerStatus *string 1.17.0 event_scheduler,omitempty
Expand Down
45 changes: 18 additions & 27 deletions go/libraries/doltcore/servercfg/yaml_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func nillableIntPtr(n int) *int {
type BehaviorYAMLConfig struct {
ReadOnly *bool `yaml:"read_only"`
AutoCommit *bool `yaml:"autocommit"`
// PersistenceBehavior regulates loading persisted system variable configuration.
PersistenceBehavior *string `yaml:"persistence_behavior"`
// PersistenceBehavior is unused, but still present to prevent breaking any YAML configs that still use it.
PersistenceBehavior *string `yaml:"persistence_behavior,omitempty"`
// Disable processing CLIENT_MULTI_STATEMENTS support on the
// sql server. Dolt's handling of CLIENT_MULTI_STATEMENTS is currently
// broken. If a client advertises to support it (mysql cli client
Expand Down Expand Up @@ -174,34 +174,33 @@ func YamlConfigFromFile(fs filesys.Filesys, path string) (ServerConfig, error) {
}

func ServerConfigAsYAMLConfig(cfg ServerConfig) *YAMLConfig {
systemVars := map[string]interface{}(cfg.SystemVars())
systemVars := cfg.SystemVars()
return &YAMLConfig{
LogLevelStr: ptr(string(cfg.LogLevel())),
MaxQueryLenInLogs: nillableIntPtr(cfg.MaxLoggedQueryLen()),
EncodeLoggedQuery: nillableBoolPtr(cfg.ShouldEncodeLoggedQuery()),
BehaviorConfig: BehaviorYAMLConfig{
ptr(cfg.ReadOnly()),
ptr(cfg.AutoCommit()),
ptr(cfg.PersistenceBehavior()),
ptr(cfg.DisableClientMultiStatements()),
ptr(cfg.DoltTransactionCommit()),
ptr(cfg.EventSchedulerStatus()),
ReadOnly: ptr(cfg.ReadOnly()),
AutoCommit: ptr(cfg.AutoCommit()),
DisableClientMultiStatements: ptr(cfg.DisableClientMultiStatements()),
DoltTransactionCommit: ptr(cfg.DoltTransactionCommit()),
EventSchedulerStatus: ptr(cfg.EventSchedulerStatus()),
},
UserConfig: UserYAMLConfig{
Name: ptr(cfg.User()),
Password: ptr(cfg.Password()),
},
ListenerConfig: ListenerYAMLConfig{
ptr(cfg.Host()),
ptr(cfg.Port()),
ptr(cfg.MaxConnections()),
ptr(cfg.ReadTimeout()),
ptr(cfg.WriteTimeout()),
nillableStrPtr(cfg.TLSKey()),
nillableStrPtr(cfg.TLSCert()),
nillableBoolPtr(cfg.RequireSecureTransport()),
nillableBoolPtr(cfg.AllowCleartextPasswords()),
nillableStrPtr(cfg.Socket()),
HostStr: ptr(cfg.Host()),
PortNumber: ptr(cfg.Port()),
MaxConnections: ptr(cfg.MaxConnections()),
ReadTimeoutMillis: ptr(cfg.ReadTimeout()),
WriteTimeoutMillis: ptr(cfg.WriteTimeout()),
TLSKey: nillableStrPtr(cfg.TLSKey()),
TLSCert: nillableStrPtr(cfg.TLSCert()),
RequireSecureTransport: nillableBoolPtr(cfg.RequireSecureTransport()),
AllowCleartextPasswords: nillableBoolPtr(cfg.AllowCleartextPasswords()),
Socket: nillableStrPtr(cfg.Socket()),
},
PerformanceConfig: PerformanceYAMLConfig{
QueryParallelism: nillableIntPtr(cfg.QueryParallelism()),
Expand Down Expand Up @@ -528,14 +527,6 @@ func (cfg YAMLConfig) ShouldEncodeLoggedQuery() bool {
return *cfg.EncodeLoggedQuery
}

// PersistenceBehavior is "load" if we include persisted system globals on server init
func (cfg YAMLConfig) PersistenceBehavior() string {
if cfg.BehaviorConfig.PersistenceBehavior == nil {
return LoadPerisistentGlobals
}
return *cfg.BehaviorConfig.PersistenceBehavior
}

// DataDir is the path to a directory to use as the data dir, both to create new databases and locate existing ones.
func (cfg YAMLConfig) DataDir() string {
if cfg.DataDirStr != nil {
Expand Down
1 change: 0 additions & 1 deletion go/libraries/doltcore/servercfg/yaml_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ behavior:
read_only: false
autocommit: true
dolt_transaction_commit: true
persistence_behavior: load
disable_client_multi_statements: false
event_scheduler: ON

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,30 +211,18 @@ tests:
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["999"]]
- name: persistence behavior set to load
- name: persistence behavior
repos:
- name: repo1
server:
args: ["--persistence-behavior", "load"]
args: []
connections:
- on: repo1
queries:
- query: "select @@GLOBAL.max_connections"
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["151"]]
- name: persistence behavior set to ignore
repos:
- name: repo1
server:
args: ["--persistence-behavior", "ignore"]
connections:
- on: repo1
queries:
- query: "select @@GLOBAL.max_connections"
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["100"]]
- name: persisted global variable defined on the command line
repos:
- name: repo1
Expand All @@ -247,34 +235,6 @@ tests:
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["555"]]
- name: persist global variable before server startup with persistence behavior with ignore
repos:
- name: repo1
with_files:
- name: ".dolt/config.json"
contents: |
{"sqlserver.global.max_connections":"999"}
server:
args: ["--persistence-behavior", "ignore"]
connections:
- on: repo1
queries:
- query: "select @@GLOBAL.max_connections"
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["100"]]
- name: persisted global variable defined on the command line with persistence ignored
repos:
- name: repo1
server:
args: ["--max-connections", "555", "--persistence-behavior", "ignore"]
connections:
- on: repo1
queries:
- query: "select @@GLOBAL.max_connections"
result:
columns: ["@@GLOBAL.max_connections"]
rows: [["555"]]
- name: "@@global.dolt_log_level behavior"
repos:
- name: repo1
Expand Down
Loading