From 2146ed4033f6288ea85eea454a0be14ace201c7f Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Tue, 10 Jan 2023 08:07:45 +0100 Subject: [PATCH] xl: Quit early when EC config is incorrect (#16390) Co-authored-by: Anis Elleuch --- cmd/erasure-server-pool.go | 7 +++++-- cmd/erasure-sets.go | 4 ---- cmd/erasure-sets_test.go | 7 ++++++- cmd/format-erasure.go | 16 +++++++++------- cmd/server-main.go | 5 +++++ internal/config/storageclass/storage-class.go | 7 +------ 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index f0ac9d368..80062069b 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -94,11 +94,14 @@ func newErasureServerPools(ctx context.Context, endpointServerPools EndpointServ // -- Default for Standard Storage class is, parity = 3 - disks 6, 7 // -- Default for Standard Storage class is, parity = 4 - disks 8 to 16 if commonParityDrives == 0 { - commonParityDrives = ecDrivesNoConfig(ep.DrivesPerSet) + commonParityDrives, err = ecDrivesNoConfig(ep.DrivesPerSet) + if err != nil { + return nil, err + } } if err = storageclass.ValidateParity(commonParityDrives, ep.DrivesPerSet); err != nil { - return nil, fmt.Errorf("parity validation returned an error %w <- (%d, %d), for pool(%s)", err, commonParityDrives, ep.DrivesPerSet, humanize.Ordinal(i+1)) + return nil, fmt.Errorf("parity validation returned an error: %w <- (%d, %d), for pool(%s)", err, commonParityDrives, ep.DrivesPerSet, humanize.Ordinal(i+1)) } storageDisks[i], formats[i], err = waitForFormatErasure(local, ep.Endpoints, i+1, diff --git a/cmd/erasure-sets.go b/cmd/erasure-sets.go index 17df60854..2139c05bc 100644 --- a/cmd/erasure-sets.go +++ b/cmd/erasure-sets.go @@ -354,10 +354,6 @@ func newErasureSets(ctx context.Context, endpoints PoolEndpoints, storageDisks [ endpointStrings[i] = endpoint.String() } - if defaultParityCount == 0 { - logger.Error("Warning: Default parity set to 0. This can lead to data loss.") - } - // Initialize the erasure sets instance. s := &erasureSets{ sets: make([]*erasureObjects, setCount), diff --git a/cmd/erasure-sets_test.go b/cmd/erasure-sets_test.go index e22658a37..9faa83f9d 100644 --- a/cmd/erasure-sets_test.go +++ b/cmd/erasure-sets_test.go @@ -191,7 +191,12 @@ func TestNewErasureSets(t *testing.T) { } ep := PoolEndpoints{Endpoints: endpoints} - if _, err := newErasureSets(ctx, ep, storageDisks, format, ecDrivesNoConfig(16), 0); err != nil { + + parity, err := ecDrivesNoConfig(16) + if err != nil { + t.Fatalf("Unexpected error during EC drive config: %v", err) + } + if _, err := newErasureSets(ctx, ep, storageDisks, format, parity, 0); err != nil { t.Fatalf("Unable to initialize erasure") } } diff --git a/cmd/format-erasure.go b/cmd/format-erasure.go index ed7d1dd7b..4540497a9 100644 --- a/cmd/format-erasure.go +++ b/cmd/format-erasure.go @@ -741,7 +741,10 @@ func fixFormatErasureV3(storageDisks []StorageAPI, endpoints Endpoints, formats func initFormatErasure(ctx context.Context, storageDisks []StorageAPI, setCount, setDriveCount int, deploymentID, distributionAlgo string, sErrs []error) (*formatErasureV3, error) { format := newFormatErasureV3(setCount, setDriveCount) formats := make([]*formatErasureV3, len(storageDisks)) - wantAtMost := ecDrivesNoConfig(setDriveCount) + wantAtMost, err := ecDrivesNoConfig(setDriveCount) + if err != nil { + return nil, err + } for i := 0; i < setCount; i++ { hostCount := make(map[string]int, setDriveCount) @@ -795,13 +798,12 @@ func initFormatErasure(ctx context.Context, storageDisks []StorageAPI, setCount, // ecDrivesNoConfig returns the erasure coded drives in a set if no config has been set. // It will attempt to read it from env variable and fall back to drives/2. -func ecDrivesNoConfig(setDriveCount int) int { - sc, _ := storageclass.LookupConfig(config.KVS{}, setDriveCount) - ecDrives := sc.GetParityForSC(storageclass.STANDARD) - if ecDrives < 0 { - ecDrives = storageclass.DefaultParityBlocks(setDriveCount) +func ecDrivesNoConfig(setDriveCount int) (int, error) { + sc, err := storageclass.LookupConfig(config.KVS{}, setDriveCount) + if err != nil { + return 0, err } - return ecDrives + return sc.GetParityForSC(storageclass.STANDARD), nil } // Make Erasure backend meta volumes. diff --git a/cmd/server-main.go b/cmd/server-main.go index 98c77c302..e7ac1d1f5 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -722,6 +722,11 @@ func serverMain(ctx *cli.Context) { // Prints the formatted startup message, if err is not nil then it prints additional information as well. printStartupMessage(getAPIEndpoints(), err) + + // Print a warning at the end of the startup banner so it is more noticeable + if globalStorageClass.GetParityForSC("") == 0 { + logger.Error("Warning: The standard parity is set to 0. This can lead to data loss.") + } }() region := globalSite.Region diff --git a/internal/config/storageclass/storage-class.go b/internal/config/storageclass/storage-class.go index eb1b871e0..ba6d69e43 100644 --- a/internal/config/storageclass/storage-class.go +++ b/internal/config/storageclass/storage-class.go @@ -304,12 +304,7 @@ func LookupConfig(kvs config.KVS, setDriveCount int) (cfg Config, err error) { // Validation is done after parsing both the storage classes. This is needed because we need one // storage class value to deduce the correct value of the other storage class. if err = validateParity(cfg.Standard.Parity, cfg.RRS.Parity, setDriveCount); err != nil { - cfg.RRS.Parity = defaultRRSParity - if setDriveCount == 1 { - cfg.RRS.Parity = 0 - } - cfg.Standard.Parity = DefaultParityBlocks(setDriveCount) - return cfg, err + return Config{}, err } return cfg, nil