From e0aa6360e8a5c81a399f0aa1671d0164a3e49a87 Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Wed, 20 Mar 2024 16:21:01 -0400 Subject: [PATCH] storage: support WAL failover to an explicit path This commit expands on #120509, introducing a WAL failover mode that allows an operator of a node with a single store to configure WAL failover to failover to a particular path (rather than another store's directory). This is configured via the --wal-failover flag: --wal-failover=path=/mnt/data2 When disabling or changing the path, the operator is required to pass the previous path. Eg, --wal-failover=path=/mnt/data3,prev_path=/mnt/data2 or --wal-failover=disabled,prev_path=/mnt/data2 Informs #119418. Informs cockroachdb/pebble#3230 Epic: CRDB-35401 Release note (ops change): Adds an additional option to the new (in 24.1) --wal-failover CLI flag allowing an operator to specify an explicit path for WAL failover for single-store nodes. --- pkg/base/BUILD.bazel | 1 + pkg/base/config.go | 124 ++++++++++++++++-- pkg/base/config_test.go | 20 +++ pkg/base/store_spec.go | 4 +- pkg/base/testdata/wal-failover-config | 29 +++++ pkg/ccl/baseccl/encryption_spec.go | 60 ++++++--- pkg/ccl/cliccl/debug.go | 16 +-- pkg/ccl/cliccl/start.go | 15 ++- pkg/cli/cliflags/flags.go | 27 +++- pkg/cli/context.go | 10 ++ pkg/cli/flags.go | 21 +++- pkg/cli/flags_test.go | 2 +- pkg/server/config.go | 4 +- pkg/storage/fs/fs.go | 3 + pkg/storage/open.go | 175 +++++++++++++++++++++----- 15 files changed, 424 insertions(+), 87 deletions(-) create mode 100644 pkg/base/testdata/wal-failover-config diff --git a/pkg/base/BUILD.bazel b/pkg/base/BUILD.bazel index 4f7102949356..3517bbde2077 100644 --- a/pkg/base/BUILD.bazel +++ b/pkg/base/BUILD.bazel @@ -69,6 +69,7 @@ go_test( "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/uuid", + "@com_github_cockroachdb_datadriven//:datadriven", "@com_github_cockroachdb_errors//:errors", "@com_github_davecgh_go_spew//spew", "@com_github_stretchr_testify//require", diff --git a/pkg/base/config.go b/pkg/base/config.go index a053adcafb7a..e7af4e91380c 100644 --- a/pkg/base/config.go +++ b/pkg/base/config.go @@ -17,6 +17,7 @@ import ( "net" "net/url" "os" + "strings" "time" "github.com/cockroachdb/cockroach/pkg/roachpb" @@ -838,8 +839,7 @@ type TempStorageConfig struct { Settings *cluster.Settings } -// WALFailoverMode configures a node's stores behavior under high write latency -// to their write-ahead logs. +// WALFailoverMode specifies the mode of WAL failover. type WALFailoverMode int8 const ( @@ -858,11 +858,11 @@ const ( // volume, the batch commit latency is insulated from the effects of momentary // disk stalls. WALFailoverAmongStores + // WALFailoverExplicitPath enables WAL failover for a single-store node to an + // explicitly specified path. + WALFailoverExplicitPath ) -// Type implements the pflag.Value interface. -func (m *WALFailoverMode) Type() string { return "string" } - // String implements fmt.Stringer. func (m *WALFailoverMode) String() string { return redact.StringWithoutMarkers(m) @@ -877,25 +877,125 @@ func (m *WALFailoverMode) SafeFormat(p redact.SafePrinter, _ rune) { p.SafeString("disabled") case WALFailoverAmongStores: p.SafeString("among-stores") + case WALFailoverExplicitPath: + p.SafeString("path") default: p.Printf("", int8(*m)) } } +// WALFailoverConfig configures a node's stores behavior under high write +// latency to their write-ahead logs. +type WALFailoverConfig struct { + Mode WALFailoverMode + // Path is the non-store path to which WALs should be written when failing + // over. It must be nonempty if and only if Mode == WALFailoverExplicitPath. + Path ExternalPath + // PrevPath is the previously used non-store path. It may be set with Mode == + // WALFailoverExplicitPath (when changing the secondary path) or + // WALFailoverDisabled (when disabling WAL failover after it was previously + // enabled with WALFailoverExplicitPath). It must be empty for other modes. + // If Mode is WALFailoverDisabled and previously WAL failover was enabled + // using WALFailoverAmongStores, then PrevPath must not be set. + PrevPath ExternalPath +} + +// ExternalPath represents a non-store path and associated encryption-at-rest +// configuration. +type ExternalPath struct { + Path string + // EncryptionOptions is a serialized protobuf set by Go CCL code describing + // the encryption-at-rest configuration. If encryption-at-rest has ever been + // enabled on the store, this field must be set. + EncryptionOptions []byte +} + +// IsSet returns whether or not the external path was provided. +func (e ExternalPath) IsSet() bool { return e.Path != "" } + +// Type implements the pflag.Value interface. +func (c *WALFailoverConfig) Type() string { return "string" } + +// String implements fmt.Stringer. +func (c *WALFailoverConfig) String() string { + return redact.StringWithoutMarkers(c) +} + +// SafeFormat implements the refact.SafeFormatter interface. +func (c *WALFailoverConfig) SafeFormat(p redact.SafePrinter, _ rune) { + switch c.Mode { + case WALFailoverDefault: + // Empty + case WALFailoverDisabled: + p.SafeString("disabled") + if c.PrevPath.IsSet() { + p.SafeString(",prev_path=") + p.SafeString(redact.SafeString(c.PrevPath.Path)) + } + case WALFailoverAmongStores: + p.SafeString("among-stores") + case WALFailoverExplicitPath: + p.SafeString("path=") + p.SafeString(redact.SafeString(c.Path.Path)) + if c.PrevPath.IsSet() { + p.SafeString(",prev_path=") + p.SafeString(redact.SafeString(c.PrevPath.Path)) + } + default: + p.Printf("", int8(c.Mode)) + } +} + // Set implements the pflag.Value interface. -func (m *WALFailoverMode) Set(s string) error { - switch s { - case "disabled": - *m = WALFailoverDisabled - case "among-stores": - *m = WALFailoverAmongStores +func (c *WALFailoverConfig) Set(s string) error { + switch { + case strings.HasPrefix(s, "disabled"): + c.Mode = WALFailoverDisabled + var ok bool + c.Path.Path, c.PrevPath.Path, ok = parseWALFailoverPathFields(strings.TrimPrefix(s, "disabled")) + if !ok || c.Path.IsSet() { + return errors.Newf("invalid disabled --wal-failover setting: %s "+ + "expect disabled[,prev_path=]", s) + } + case s == "among-stores": + c.Mode = WALFailoverAmongStores + case strings.HasPrefix(s, "path="): + c.Mode = WALFailoverExplicitPath + var ok bool + c.Path.Path, c.PrevPath.Path, ok = parseWALFailoverPathFields(s) + if !ok || !c.Path.IsSet() { + return errors.Newf("invalid path --wal-failover setting: %s "+ + "expect path=[,prev_path=]", s) + } default: return errors.Newf("invalid --wal-failover setting: %s "+ - "(possible values: disabled, among-stores)", s) + "(possible values: disabled, among-stores, path=)", s) } return nil } +func parseWALFailoverPathFields(s string) (path, prevPath string, ok bool) { + if s == "" { + return "", "", true + } + if s2 := strings.TrimPrefix(s, "path="); len(s2) < len(s) { + s = s2 + if i := strings.IndexByte(s, ','); i == -1 { + return s, "", true + } else { + path = s[:i] + s = s[i:] + } + } + + // Any remainder must be a prev_path= field. + if !strings.HasPrefix(s, ",prev_path=") { + return "", "", false + } + prevPath = strings.TrimPrefix(s, ",prev_path=") + return path, prevPath, true +} + // ExternalIODirConfig describes various configuration options pertaining // to external storage implementations. // TODO(adityamaru): Rename ExternalIODirConfig to ExternalIOConfig because it diff --git a/pkg/base/config_test.go b/pkg/base/config_test.go index ba1b459eee8c..21c24258cdd6 100644 --- a/pkg/base/config_test.go +++ b/pkg/base/config_test.go @@ -11,8 +11,10 @@ package base_test import ( + "bytes" "fmt" "math" + "strings" "testing" "time" @@ -20,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" "github.com/cockroachdb/cockroach/pkg/testutils/echotest" "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/datadriven" "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/require" ) @@ -154,3 +157,20 @@ func TestRaftMaxInflightBytes(t *testing.T) { }) } } + +func TestWALFailoverConfigRoundtrip(t *testing.T) { + defer leaktest.AfterTest(t)() + + datadriven.RunTest(t, datapathutils.TestDataPath(t, "wal-failover-config"), func(t *testing.T, d *datadriven.TestData) string { + var buf bytes.Buffer + for _, l := range strings.Split(d.Input, "\n") { + var cfg base.WALFailoverConfig + if err := cfg.Set(l); err != nil { + fmt.Fprintf(&buf, "err: %s\n", err) + continue + } + fmt.Fprintln(&buf, cfg.String()) + } + return buf.String() + }) +} diff --git a/pkg/base/store_spec.go b/pkg/base/store_spec.go index b425317463b4..10deec96fec6 100644 --- a/pkg/base/store_spec.go +++ b/pkg/base/store_spec.go @@ -43,10 +43,10 @@ import ( // hard coded to 640MiB. const MinimumStoreSize = 10 * 64 << 20 -// GetAbsoluteStorePath takes a (possibly relative) and returns the absolute path. +// GetAbsoluteFSPath takes a (possibly relative) and returns the absolute path. // Returns an error if the path begins with '~' or Abs fails. // 'fieldName' is used in error strings. -func GetAbsoluteStorePath(fieldName string, p string) (string, error) { +func GetAbsoluteFSPath(fieldName string, p string) (string, error) { if p[0] == '~' { return "", fmt.Errorf("%s cannot start with '~': %s", fieldName, p) } diff --git a/pkg/base/testdata/wal-failover-config b/pkg/base/testdata/wal-failover-config new file mode 100644 index 000000000000..34480f7414ff --- /dev/null +++ b/pkg/base/testdata/wal-failover-config @@ -0,0 +1,29 @@ +parse +among-stores +---- +among-stores + +parse +disabled +disabled,prev_path=foo +---- +disabled +disabled,prev_path=foo + +parse +path=/foo +path=/foo,prev_path=/bar +---- +path=/foo +path=/foo,prev_path=/bar + +parse +disabled,path=foo +among-stores,path=foo +among-stores,prev_path=foo +garbage +---- +err: invalid disabled --wal-failover setting: disabled,path=foo expect disabled[,prev_path=] +err: invalid --wal-failover setting: among-stores,path=foo (possible values: disabled, among-stores, path=) +err: invalid --wal-failover setting: among-stores,prev_path=foo (possible values: disabled, among-stores, path=) +err: invalid --wal-failover setting: garbage (possible values: disabled, among-stores, path=) diff --git a/pkg/ccl/baseccl/encryption_spec.go b/pkg/ccl/baseccl/encryption_spec.go index 65a0468522f0..6defa2385ef4 100644 --- a/pkg/ccl/baseccl/encryption_spec.go +++ b/pkg/ccl/baseccl/encryption_spec.go @@ -92,7 +92,7 @@ func NewStoreEncryptionSpec(value string) (StoreEncryptionSpec, error) { switch field { case pathField: var err error - es.Path, err = base.GetAbsoluteStorePath(pathField, value) + es.Path, err = base.GetAbsoluteFSPath(pathField, value) if err != nil { return StoreEncryptionSpec{}, err } @@ -101,7 +101,7 @@ func NewStoreEncryptionSpec(value string) (StoreEncryptionSpec, error) { es.KeyPath = plaintextFieldValue } else { var err error - es.KeyPath, err = base.GetAbsoluteStorePath("key", value) + es.KeyPath, err = base.GetAbsoluteFSPath("key", value) if err != nil { return StoreEncryptionSpec{}, err } @@ -111,7 +111,7 @@ func NewStoreEncryptionSpec(value string) (StoreEncryptionSpec, error) { es.OldKeyPath = plaintextFieldValue } else { var err error - es.OldKeyPath, err = base.GetAbsoluteStorePath("old-key", value) + es.OldKeyPath, err = base.GetAbsoluteFSPath("old-key", value) if err != nil { return StoreEncryptionSpec{}, err } @@ -141,17 +141,17 @@ func NewStoreEncryptionSpec(value string) (StoreEncryptionSpec, error) { return es, nil } -// StoreEncryptionSpecList contains a slice of StoreEncryptionSpecs that implements pflag's value +// EncryptionSpecList contains a slice of StoreEncryptionSpecs that implements pflag's value // interface. -type StoreEncryptionSpecList struct { +type EncryptionSpecList struct { Specs []StoreEncryptionSpec } -var _ pflag.Value = &StoreEncryptionSpecList{} +var _ pflag.Value = &EncryptionSpecList{} // String returns a string representation of all the StoreEncryptionSpecs. This is part // of pflag's value interface. -func (encl StoreEncryptionSpecList) String() string { +func (encl EncryptionSpecList) String() string { var buffer bytes.Buffer for _, ss := range encl.Specs { fmt.Fprintf(&buffer, "--%s=%s ", cliflagsccl.EnterpriseEncryption.Name, ss) @@ -165,13 +165,13 @@ func (encl StoreEncryptionSpecList) String() string { // Type returns the underlying type in string form. This is part of pflag's // value interface. -func (encl *StoreEncryptionSpecList) Type() string { - return "StoreEncryptionSpec" +func (encl *EncryptionSpecList) Type() string { + return "EncryptionSpec" } // Set adds a new value to the StoreEncryptionSpecValue. It is the important part of // pflag's value interface. -func (encl *StoreEncryptionSpecList) Set(value string) error { +func (encl *EncryptionSpecList) Set(value string) error { spec, err := NewStoreEncryptionSpec(value) if err != nil { return err @@ -184,12 +184,13 @@ func (encl *StoreEncryptionSpecList) Set(value string) error { return nil } -// PopulateStoreSpecWithEncryption iterates through the StoreEncryptionSpecList and looks -// for matching paths in the StoreSpecList. -// Any unmatched StoreEncryptionSpec causes an error. -// Matching stores have a few encryption-related fields set. -func PopulateStoreSpecWithEncryption( - storeSpecs base.StoreSpecList, encryptionSpecs StoreEncryptionSpecList, +// PopulateWithEncryptionOpts iterates through the EncryptionSpecList and looks +// for matching paths in the StoreSpecList and WAL failover config. Any +// unmatched EncryptionSpec causes an error. +func PopulateWithEncryptionOpts( + storeSpecs base.StoreSpecList, + walFailoverConfig *base.WALFailoverConfig, + encryptionSpecs EncryptionSpecList, ) error { for _, es := range encryptionSpecs.Specs { var found bool @@ -212,8 +213,29 @@ func PopulateStoreSpecWithEncryption( found = true break } + + for _, externalPath := range [2]*base.ExternalPath{&walFailoverConfig.Path, &walFailoverConfig.PrevPath} { + if !externalPath.IsSet() || externalPath.Path != es.Path { + continue + } + // NB: The external paths WALFailoverConfig.Path and + // WALFailoverConfig.PrevPath are only ever set in single-store + // configurations. In multi-store with among-stores failover mode, these + // will be empty (so we won't encounter the same path twice). + if len(externalPath.EncryptionOptions) > 0 { + return fmt.Errorf("WAL failover path %s already has an encryption setting", + externalPath.Path) + } + opts, err := es.ToEncryptionOptions() + if err != nil { + return err + } + externalPath.EncryptionOptions = opts + found = true + } + if !found { - return fmt.Errorf("no store with path %s found for encryption setting: %v", es.Path, es) + return fmt.Errorf("no usage of path %s found for encryption setting: %v", es.Path, es) } } return nil @@ -221,9 +243,7 @@ func PopulateStoreSpecWithEncryption( // EncryptionOptionsForStore takes a store directory and returns its EncryptionOptions // if a matching entry if found in the StoreEncryptionSpecList. -func EncryptionOptionsForStore( - dir string, encryptionSpecs StoreEncryptionSpecList, -) ([]byte, error) { +func EncryptionOptionsForStore(dir string, encryptionSpecs EncryptionSpecList) ([]byte, error) { // We need an absolute path, but the input may have come in relative. path, err := filepath.Abs(dir) if err != nil { diff --git a/pkg/ccl/cliccl/debug.go b/pkg/ccl/cliccl/debug.go index 461a3beba3db..c17b4a94ee35 100644 --- a/pkg/ccl/cliccl/debug.go +++ b/pkg/ccl/cliccl/debug.go @@ -127,33 +127,33 @@ mode in the current environment. // Add the encryption flag to commands that need it. // For the encryption-status command. f := encryptionStatusCmd.Flags() - cliflagcfg.VarFlag(f, &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + cliflagcfg.VarFlag(f, &encryptionSpecs, cliflagsccl.EnterpriseEncryption) // And other flags. f.BoolVar(&encryptionStatusOpts.activeStoreIDOnly, "active-store-key-id-only", false, "print active store key ID and exit") // For the encryption-decrypt command. f = encryptionDecryptCmd.Flags() - cliflagcfg.VarFlag(f, &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + cliflagcfg.VarFlag(f, &encryptionSpecs, cliflagsccl.EnterpriseEncryption) // For the encryption-registry-list command. f = encryptionRegistryList.Flags() - cliflagcfg.VarFlag(f, &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + cliflagcfg.VarFlag(f, &encryptionSpecs, cliflagsccl.EnterpriseEncryption) // Add encryption flag to all OSS debug commands that want it. for _, cmd := range cli.DebugCommandsRequiringEncryption { - // storeEncryptionSpecs is in start.go. - cliflagcfg.VarFlag(cmd.Flags(), &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + // encryptionSpecs is in start.go. + cliflagcfg.VarFlag(cmd.Flags(), &encryptionSpecs, cliflagsccl.EnterpriseEncryption) } // init has already run in cli/debug.go since this package imports it, so // DebugPebbleCmd already has all its subcommands. We could traverse those // here. But we don't need to by using PersistentFlags. cliflagcfg.VarFlag(cli.DebugPebbleCmd.PersistentFlags(), - &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + &encryptionSpecs, cliflagsccl.EnterpriseEncryption) cli.PopulateEnvConfigHook = fillEncryptionOptionsForStore cli.EncryptedStorePathsHook = func() []string { var res []string - for _, spec := range storeEncryptionSpecs.Specs { + for _, spec := range encryptionSpecs.Specs { res = append(res, spec.Path) } return res @@ -163,7 +163,7 @@ mode in the current environment. // fillEncryptionOptionsForStore fills the EnvConfig fields // based on the --enterprise-encryption flag value. func fillEncryptionOptionsForStore(dir string, cfg *fs.EnvConfig) error { - opts, err := baseccl.EncryptionOptionsForStore(dir, storeEncryptionSpecs) + opts, err := baseccl.EncryptionOptionsForStore(dir, encryptionSpecs) if err != nil { return err } diff --git a/pkg/ccl/cliccl/start.go b/pkg/ccl/cliccl/start.go index 472e3f868932..2dd6f0e86e24 100644 --- a/pkg/ccl/cliccl/start.go +++ b/pkg/ccl/cliccl/start.go @@ -19,11 +19,11 @@ import ( // This does not define a `start` command, only modifications to the existing command // in `pkg/cli/start.go`. -var storeEncryptionSpecs baseccl.StoreEncryptionSpecList +var encryptionSpecs baseccl.EncryptionSpecList func init() { for _, cmd := range cli.StartCmds { - cliflagcfg.VarFlag(cmd.Flags(), &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption) + cliflagcfg.VarFlag(cmd.Flags(), &encryptionSpecs, cliflagsccl.EnterpriseEncryption) // Add a new pre-run command to match encryption specs to store specs. cli.AddPersistentPreRunE(cmd, func(cmd *cobra.Command, _ []string) error { @@ -32,8 +32,13 @@ func init() { } } -// populateStoreSpecsEncryption is a PreRun hook that matches store encryption specs with the -// parsed stores and populates some fields in the StoreSpec. +// populateStoreSpecsEncryption is a PreRun hook that matches store encryption +// specs with the parsed stores and populates some fields in the StoreSpec and +// WAL failover config. func populateStoreSpecsEncryption() error { - return baseccl.PopulateStoreSpecWithEncryption(cli.GetServerCfgStores(), storeEncryptionSpecs) + return baseccl.PopulateWithEncryptionOpts( + cli.GetServerCfgStores(), + cli.GetWALFailoverConfig(), + encryptionSpecs, + ) } diff --git a/pkg/cli/cliflags/flags.go b/pkg/cli/cliflags/flags.go index 55769680bde1..d2ca2cd83d5a 100644 --- a/pkg/cli/cliflags/flags.go +++ b/pkg/cli/cliflags/flags.go @@ -1005,15 +1005,34 @@ which use 'cockroach-data-tenant-X' for tenant 'X') Name: "wal-failover", EnvVar: "COCKROACH_WAL_FAILOVER", Description: ` -Configures the use and behavior of WAL failover. Defaults to "disabled". -The value "among-stores" enables automatic failover to another store's -data directory if a WAL write does not complete within the configured -threshold. For example: +Configures the use and behavior of WAL failover. WAL failover enables +automatic failover to another directory if a WAL write does not complete +within the configured threshold. Defaults to "disabled". Possible values +depend on the number of stores a node is configured to use. + +If a node has multiple stores, the value "among-stores" enables automatic +failover to another store's data directory. CockroachDB will automatically +assign each store a secondary to serve as its WAL failover destination. +For example: 
 
   --wal-failover=among-stores
+ +If a node has a single store, the value "path=" enables automatic +failover to the provided path. After this setting is used, changing the +configuration to a new path or disabling requires providing the previous +path as ",prev_path=". For example: + +
+
+    --wal-failover=path=/mnt/data2
+    --wal-failover=path=/mnt/data3,prev_path=/mnt/data2
+    --wal-failover=disabled,prev_path=/mnt/data3
+
+
+ See the storage.wal_failover.unhealthy_op_threshold cluster setting. `, } diff --git a/pkg/cli/context.go b/pkg/cli/context.go index f53cc863b62d..1d8ffd02f499 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -733,3 +733,13 @@ func setUserfileContextDefaults() { func GetServerCfgStores() base.StoreSpecList { return serverCfg.Stores } + +// GetWALFailoverConfig provides direct public access to the WALFailoverConfig +// inside serverCfg. This is used by CCL code to populate some fields. +// +// WARNING: consider very carefully whether you should be using this. +// If you are not writing CCL code that performs command-line flag +// parsing, you probably should not be using this. +func GetWALFailoverConfig() *base.WALFailoverConfig { + return &serverCfg.WALFailover +} diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index ff70ebdbdb16..57e6ceacd84d 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -1249,14 +1249,14 @@ func extraStoreFlagInit(cmd *cobra.Command) error { } // Convert all the store paths to absolute paths. We want this to // ensure canonical directories across invocations; and also to - // benefit from the check in GetAbsoluteStorePath() that the user + // benefit from the check in GetAbsoluteFSPath() that the user // didn't mistakenly assume a heading '~' would get translated by // CockroachDB. (The shell should be responsible for that.) for i, ss := range serverCfg.Stores.Specs { if ss.InMemory { continue } - absPath, err := base.GetAbsoluteStorePath("path", ss.Path) + absPath, err := base.GetAbsoluteFSPath("path", ss.Path) if err != nil { return err } @@ -1264,6 +1264,21 @@ func extraStoreFlagInit(cmd *cobra.Command) error { serverCfg.Stores.Specs[i] = ss } + if serverCfg.WALFailover.Path.IsSet() { + absPath, err := base.GetAbsoluteFSPath("wal-failover.path", serverCfg.WALFailover.Path.Path) + if err != nil { + return err + } + serverCfg.WALFailover.Path.Path = absPath + } + if serverCfg.WALFailover.PrevPath.IsSet() { + absPath, err := base.GetAbsoluteFSPath("wal-failover.prev_path", serverCfg.WALFailover.PrevPath.Path) + if err != nil { + return err + } + serverCfg.WALFailover.PrevPath.Path = absPath + } + // Configure the external I/O directory. if !fs.Changed(cliflags.ExternalIODir.Name) { // Try to find a directory from the store configuration. @@ -1278,7 +1293,7 @@ func extraStoreFlagInit(cmd *cobra.Command) error { if startCtx.externalIODir != "" { // Make the directory name absolute. var err error - startCtx.externalIODir, err = base.GetAbsoluteStorePath(cliflags.ExternalIODir.Name, startCtx.externalIODir) + startCtx.externalIODir, err = base.GetAbsoluteFSPath(cliflags.ExternalIODir.Name, startCtx.externalIODir) if err != nil { return err } diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go index d3a121c4783f..de8f89a4661a 100644 --- a/pkg/cli/flags_test.go +++ b/pkg/cli/flags_test.go @@ -1344,7 +1344,7 @@ func TestSQLPodStorageDefaults(t *testing.T) { defer initCLIDefaults() - expectedDefaultDir, err := base.GetAbsoluteStorePath("", "cockroach-data-tenant-9") + expectedDefaultDir, err := base.GetAbsoluteFSPath("", "cockroach-data-tenant-9") if err != nil { t.Fatal(err) } diff --git a/pkg/server/config.go b/pkg/server/config.go index 87c2690716fb..e28390da945f 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -230,7 +230,7 @@ type BaseConfig struct { // WALFailover enables and configures automatic WAL failover when latency to // a store's primary WAL increases. - WALFailover base.WALFailoverMode + WALFailover base.WALFailoverConfig // SharedStorage is specified to enable disaggregated shared storage. SharedStorage string @@ -316,7 +316,7 @@ func (cfg *BaseConfig) SetDefaults( cfg.DisableMaxOffsetCheck = false cfg.DefaultZoneConfig = zonepb.DefaultZoneConfig() cfg.StorageEngine = storage.DefaultStorageEngine - cfg.WALFailover = base.WALFailoverDefault + cfg.WALFailover = base.WALFailoverConfig{Mode: base.WALFailoverDefault} cfg.TestingInsecureWebAccess = disableWebLogin cfg.Stores = base.StoreSpecList{ Specs: []base.StoreSpec{storeSpec}, diff --git a/pkg/storage/fs/fs.go b/pkg/storage/fs/fs.go index 25da97cc1b5d..38e21e2824c3 100644 --- a/pkg/storage/fs/fs.go +++ b/pkg/storage/fs/fs.go @@ -228,6 +228,9 @@ func (e *Env) IsReadOnly() bool { return e.rw == ReadOnly } +// RWMode returns the read-write mode of the environment. +func (e *Env) RWMode() RWMode { return e.rw } + // RegisterOnDiskSlow configures the Env to call the provided function when a // disk operation is slow. func (e *Env) RegisterOnDiskSlow(fn func(vfs.DiskSlowInfo)) { diff --git a/pkg/storage/open.go b/pkg/storage/open.go index e7f793909bb4..ed48cca7c10a 100644 --- a/pkg/storage/open.go +++ b/pkg/storage/open.go @@ -24,6 +24,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/fs" "github.com/cockroachdb/errors" "github.com/cockroachdb/pebble" + "github.com/cockroachdb/pebble/vfs" "github.com/cockroachdb/pebble/wal" ) @@ -210,20 +211,129 @@ func LBaseMaxBytes(v int64) ConfigOption { } } +func noopConfigOption(*engineConfig) error { + return nil +} + +func errConfigOption(err error) func(*engineConfig) error { + return func(*engineConfig) error { return err } +} + +func makeExternalWALDir(engineCfg *engineConfig, externalDir base.ExternalPath) (wal.Dir, error) { + // If the store is encrypted, we require that all the WAL failover dirs also + // be encrypted so that the user doesn't accidentally leak data unencrypted + // onto the filesystem. + if engineCfg.Env.Encryption != nil && len(externalDir.EncryptionOptions) == 0 { + return wal.Dir{}, errors.Newf("must provide --enterprise-encryption flag for %q, used as WAL failover path for encrypted store %q", + externalDir.Path, engineCfg.Env.Dir) + } + if engineCfg.Env.Encryption == nil && len(externalDir.EncryptionOptions) != 0 { + return wal.Dir{}, errors.Newf("must provide --enterprise-encryption flag for store %q, specified WAL failover path %q is encrypted", + engineCfg.Env.Dir, externalDir.Path) + } + env, err := fs.InitEnv(context.Background(), vfs.Default, externalDir.Path, fs.EnvConfig{ + RW: engineCfg.Env.RWMode(), + EncryptionOptions: externalDir.EncryptionOptions, + }) + if err != nil { + return wal.Dir{}, err + } + engineCfg.onClose = append(engineCfg.onClose, func(*Pebble) { env.Close() }) + return wal.Dir{ + FS: env, + Dirname: externalDir.Path, + }, nil +} + // WALFailover configures automatic failover of the engine's write-ahead log to // another volume in the event the WAL becomes blocked on a write that does not // complete within a reasonable duration. -func WALFailover(mode base.WALFailoverMode, storeEnvs fs.Envs) ConfigOption { - // If the user specified no WAL failover setting, we default to disabling WAL - // failover and assume that the previous process did not have WAL failover - // enabled (so there's no need to populate Options.WALRecoveryDirs). If an - // operator had WAL failover enabled and now wants to disable it, they must - // explicitly set --wal-failover=disabled for the next process. - if mode == base.WALFailoverDefault || len(storeEnvs) == 1 { - return func(cfg *engineConfig) error { return nil } +func WALFailover(walCfg base.WALFailoverConfig, storeEnvs fs.Envs) ConfigOption { + // The set of options available in single-store versus multi-store + // configurations vary. This is in part due to the need to store the multiple + // stores' WALs separately. When WALFailoverExplicitPath is provided, we have + // no stable store identifier available to disambiguate the WALs of multiple + // stores. Note that the store ID is not known when a store is first opened. + if len(storeEnvs) == 1 { + switch walCfg.Mode { + case base.WALFailoverDefault, base.WALFailoverAmongStores: + return noopConfigOption + case base.WALFailoverDisabled: + // Check if the user provided an explicit previous path. If they did, they + // were previously using WALFailoverExplicitPath and are now disabling it. + // We need to add the explicilt path to WALRecoveryDirs. + if walCfg.PrevPath.IsSet() { + return func(cfg *engineConfig) error { + walDir, err := makeExternalWALDir(cfg, walCfg.PrevPath) + if err != nil { + return err + } + cfg.Opts.WALRecoveryDirs = append(cfg.Opts.WALRecoveryDirs, walDir) + return nil + } + } + // No PrevPath was provided. The user may be simply expressing their + // intent to not run with WAL failover, regardless of any future default + // values. If WAL failover was previously enabled, Open will error when it + // notices the OPTIONS file encodes a WAL failover secondary that was not + // provided to Options.WALRecoveryDirs. + return noopConfigOption + case base.WALFailoverExplicitPath: + // The user has provided an explicit path to which we should fail over WALs. + return func(cfg *engineConfig) error { + walDir, err := makeExternalWALDir(cfg, walCfg.Path) + if err != nil { + return err + } + cfg.Opts.WALFailover = makePebbleWALFailoverOptsForDir(cfg.Settings, walDir) + if walCfg.PrevPath.IsSet() { + walDir, err := makeExternalWALDir(cfg, walCfg.PrevPath) + if err != nil { + return err + } + cfg.Opts.WALRecoveryDirs = append(cfg.Opts.WALRecoveryDirs, walDir) + } + return nil + } + default: + panic("unreachable") + } + } + + switch walCfg.Mode { + case base.WALFailoverDefault: + // If the user specified no WAL failover setting, we default to disabling WAL + // failover and assume that the previous process did not have WAL failover + // enabled (so there's no need to populate Options.WALRecoveryDirs). If an + // operator had WAL failover enabled and now wants to disable it, they must + // explicitly set --wal-failover=disabled for the next process. + return noopConfigOption + case base.WALFailoverDisabled: + // Check if the user provided an explicit previous path; that's unsupported + // in multi-store configurations. + if walCfg.PrevPath.IsSet() { + return errConfigOption(errors.Newf("storage: cannot use explicit prev_path --wal-failover option with multiple stores")) + } + // No PrevPath was provided, implying that the user previously was using + // WALFailoverAmongStores. + + // Fallthrough + case base.WALFailoverExplicitPath: + // Not supported for multi-store configurations. + return errConfigOption(errors.Newf("storage: cannot use explicit path --wal-failover option with multiple stores")) + case base.WALFailoverAmongStores: + // Fallthrough + default: + panic("unreachable") } - // mode == WALFailoverDisabled or WALFailoverAmongStores. + // Either + // 1. mode == WALFailoverAmongStores + // or + // 2. mode == WALFailoverDisabled and the user previously was using + // WALFailoverAmongStores, so we should build the deterministic store pairing + // to determine which WALRecoveryDirs to pass to which engines. + // // For each store, we need to determine which store is its secondary for the // purpose of WALs. Even if failover is disabled, it's possible that it wasn't // when the previous process ran, and the secondary's wal dir may have WALs @@ -274,27 +384,8 @@ func WALFailover(mode base.WALFailoverMode, storeEnvs fs.Envs) ConfigOption { // Use auxiliary/wals-among-stores within the other stores directory. Dirname: secondaryEnv.PathJoin(secondaryEnv.Dir, base.AuxiliaryDir, "wals-among-stores"), } - - if mode == base.WALFailoverAmongStores { - cfg.Opts.WALFailover = &pebble.WALFailoverOptions{ - Secondary: secondary, - FailoverOptions: wal.FailoverOptions{ - // Leave most the options to their defaults, but - // UnhealthyOperationLatencyThreshold should be pulled from the - // cluster setting. - UnhealthyOperationLatencyThreshold: func() (time.Duration, bool) { - // WAL failover requires 24.1 to be finalized first. Otherwise, we might - // write WALs to a secondary, downgrade to a previous version's binary and - // blindly miss WALs. The second return value indicates whether the - // WAL manager is allowed to failover to the secondary. - // - // NB: We do not use settings.Version.IsActive because we do not have a - // guarantee that the cluster version has been initialized. - failoverOK := cfg.Settings.Version.ActiveVersionOrEmpty(context.TODO()).IsActive(clusterversion.V24_1Start) - return walFailoverUnhealthyOpThreshold.Get(&cfg.Settings.SV), failoverOK - }, - }, - } + if walCfg.Mode == base.WALFailoverAmongStores { + cfg.Opts.WALFailover = makePebbleWALFailoverOptsForDir(cfg.Settings, secondary) return nil } // mode == WALFailoverDisabled @@ -303,6 +394,30 @@ func WALFailover(mode base.WALFailoverMode, storeEnvs fs.Envs) ConfigOption { } } +func makePebbleWALFailoverOptsForDir( + settings *cluster.Settings, dir wal.Dir, +) *pebble.WALFailoverOptions { + return &pebble.WALFailoverOptions{ + Secondary: dir, + FailoverOptions: wal.FailoverOptions{ + // Leave most the options to their defaults, but + // UnhealthyOperationLatencyThreshold should be pulled from the + // cluster setting. + UnhealthyOperationLatencyThreshold: func() (time.Duration, bool) { + // WAL failover requires 24.1 to be finalized first. Otherwise, we might + // write WALs to a secondary, downgrade to a previous version's binary and + // blindly miss WALs. The second return value indicates whether the + // WAL manager is allowed to failover to the secondary. + // + // NB: We do not use settings.Version.IsActive because we do not have a + // guarantee that the cluster version has been initialized. + failoverOK := settings.Version.ActiveVersionOrEmpty(context.TODO()).IsActive(clusterversion.V24_1Start) + return walFailoverUnhealthyOpThreshold.Get(&settings.SV), failoverOK + }, + }, + } +} + // PebbleOptions contains Pebble-specific options in the same format as a // Pebble OPTIONS file. For example: // [Options]