diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-05-27 15:50:31 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-05-29 11:16:29 +0300 |
commit | 4fa59a2930143998f91a3849556696bb3103c009 (patch) | |
tree | 14ae43310ba46716118187efa5a1a655fe84baa8 | |
parent | a1fe1986c75312d557b73f13fe35587462c4ca5f (diff) |
failover: Default to enabling SQL strategy
Given the SQL strategy is a reasonable default, and tested well enough,
this change makes the SQL strategy the preferred strategy. This was
`local`, which creates dataloss scenarios when multiple Praefects are
behind a load balancer.
Additionally, this changes behaviour around the read only feature. This
would default to being enabled. Currently availability is favoured over
consistency. Now this is flipped.
Fixes: https://gitlab.com/gitlab-org/gitaly/-/issues/2682
-rw-r--r-- | changelogs/unreleased/zj-default-sql-failover.yml | 5 | ||||
-rw-r--r-- | config.praefect.toml.example | 4 | ||||
-rw-r--r-- | internal/praefect/config/config.go | 10 | ||||
-rw-r--r-- | internal/praefect/config/config_test.go | 22 | ||||
-rw-r--r-- | internal/praefect/config/testdata/config.overwritedefaults.toml | 5 |
5 files changed, 42 insertions, 4 deletions
diff --git a/changelogs/unreleased/zj-default-sql-failover.yml b/changelogs/unreleased/zj-default-sql-failover.yml new file mode 100644 index 000000000..7e41e9b86 --- /dev/null +++ b/changelogs/unreleased/zj-default-sql-failover.yml @@ -0,0 +1,5 @@ +--- +title: 'failover: Default to enabling SQL strategy' +merge_request: 2218 +author: +type: added diff --git a/config.praefect.toml.example b/config.praefect.toml.example index 8b768a93d..9552c868b 100644 --- a/config.praefect.toml.example +++ b/config.praefect.toml.example @@ -31,8 +31,8 @@ listen_addr = "127.0.0.1:2305" [failover] enabled = true -election_strategy = "local" # Options: local, sql -read_only_after_failover = false # Switch the virtual storage to read-only mode after after a failover. +election_strategy = "sql" # Options: local, sql. Defaults to 'sql'. +read_only_after_failover = true # Switch the virtual storage to read-only mode after after a failover. [[virtual_storage]] name = 'praefect' diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index 98d914ba9..d81430cc0 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -21,6 +21,8 @@ type Failover struct { ReadOnlyAfterFailover bool `toml:"read_only_after_failover"` } +const sqlFailoverValue = "sql" + // Config is a container for everything found in the TOML config file type Config struct { ListenAddr string `toml:"listen_addr"` @@ -48,7 +50,10 @@ type VirtualStorage struct { // FromFile loads the config for the passed file path func FromFile(filePath string) (Config, error) { - conf := &Config{} + conf := &Config{ + // Sets the default Failover, to be overwritten when deserializing the TOML + Failover: Failover{Enabled: true, ElectionStrategy: sqlFailoverValue, ReadOnlyAfterFailover: true}, + } if _, err := toml.DecodeFile(filePath, conf); err != nil { return Config{}, err } @@ -145,7 +150,8 @@ func (c *Config) Validate() error { // NeedsSQL returns true if the driver for SQL needs to be initialized func (c *Config) NeedsSQL() bool { - return !c.MemoryQueueEnabled || (c.Failover.Enabled && c.Failover.ElectionStrategy == "sql") + return !c.MemoryQueueEnabled || + (c.Failover.Enabled && c.Failover.ElectionStrategy == sqlFailoverValue) } func (c *Config) setDefaults() { diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 9fff7b1d3..0537cdfb1 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -266,6 +266,23 @@ func TestConfigParsing(t *testing.T) { }, MemoryQueueEnabled: true, GracefulStopTimeout: config.Duration(30 * time.Second), + Failover: Failover{ + Enabled: true, + ElectionStrategy: sqlFailoverValue, + ReadOnlyAfterFailover: true, + }, + }, + }, + { + desc: "overwriting default values in the config", + filePath: "testdata/config.overwritedefaults.toml", + expected: Config{ + GracefulStopTimeout: config.Duration(time.Minute), + Failover: Failover{ + Enabled: false, + ElectionStrategy: "local", + ReadOnlyAfterFailover: false, + }, }, }, { @@ -273,6 +290,11 @@ func TestConfigParsing(t *testing.T) { filePath: "testdata/config.empty.toml", expected: Config{ GracefulStopTimeout: config.Duration(time.Minute), + Failover: Failover{ + Enabled: true, + ElectionStrategy: sqlFailoverValue, + ReadOnlyAfterFailover: true, + }, }, }, { diff --git a/internal/praefect/config/testdata/config.overwritedefaults.toml b/internal/praefect/config/testdata/config.overwritedefaults.toml new file mode 100644 index 000000000..39771fcd6 --- /dev/null +++ b/internal/praefect/config/testdata/config.overwritedefaults.toml @@ -0,0 +1,5 @@ +[failover] +enabled = false +election_strategy = "local" +read_only_after_failover = false + |