From 1e3d80552f97c4a15783f7718831098647ce710e Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 26 Jul 2016 03:18:47 -0700 Subject: [PATCH] XL: format.json healing should cater for mismatching order. (#2285) Fresh disks can be provided in any order, we need to make sure to preserve existing disk order and populate the fresh disks in new positions. Thanks for Anis Elleuch for finding this issue. --- format-config-v1.go | 82 +++++++++++++++++++++++++++++---------------- xl-v1.go | 2 +- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/format-config-v1.go b/format-config-v1.go index fcbb47c49..cacd51aa8 100644 --- a/format-config-v1.go +++ b/format-config-v1.go @@ -20,7 +20,7 @@ import ( "encoding/json" "errors" "fmt" - "strings" + "reflect" "sync" ) @@ -296,21 +296,21 @@ func checkDisksConsistency(formatConfigs []*formatConfigV1) error { // checkJBODConsistency - validate xl jbod order if they are consistent. func checkJBODConsistency(formatConfigs []*formatConfigV1) error { - var jbodStr string + var sentinelJBOD []string // Extract first valid JBOD. for _, format := range formatConfigs { if format == nil { continue } - jbodStr = strings.Join(format.XL.JBOD, ".") + sentinelJBOD = format.XL.JBOD break } for _, format := range formatConfigs { if format == nil { continue } - savedJBODStr := strings.Join(format.XL.JBOD, ".") - if jbodStr != savedJBODStr { + currentJBOD := format.XL.JBOD + if !reflect.DeepEqual(sentinelJBOD, currentJBOD) { return errors.New("Inconsistent JBOD found.") } } @@ -412,14 +412,15 @@ func isFormatFound(formats []*formatConfigV1) bool { // Heals any missing format.json on the drives. Returns error only for unexpected errors // as regular errors can be ignored since there might be enough quorum to be operational. -func healFormatXL(storageDisks []StorageAPI) error { +// Heals only fresh disks. +func healFormatXLFreshDisks(storageDisks []StorageAPI) error { formatConfigs := make([]*formatConfigV1, len(storageDisks)) var referenceConfig *formatConfigV1 // Loads `format.json` from all disks. for index, disk := range storageDisks { // Disk not found or ignored is a valid case. if disk == nil { - // Proceed without healing. + // Return nil, one of the disk is offline. return nil } formatXL, err := loadFormat(disk) @@ -436,6 +437,7 @@ func healFormatXL(storageDisks []StorageAPI) error { } // Success. formatConfigs[index] = formatXL } + // All `format.json` has been read successfully, previously completed. if isFormatFound(formatConfigs) { // Return success. @@ -446,6 +448,7 @@ func healFormatXL(storageDisks []StorageAPI) error { if isFormatNotFound(formatConfigs) { return initFormatXL(storageDisks) } + // Validate format configs for consistency in JBOD and disks. if err := checkFormatXL(formatConfigs); err != nil { return err @@ -462,40 +465,61 @@ func healFormatXL(storageDisks []StorageAPI) error { } } - // Collect new format configs. - var newFormatConfigs = make([]*formatConfigV1, len(storageDisks)) - // Collect new JBOD. newJBOD := referenceConfig.XL.JBOD - // This section heals the format.json and updates the fresh disks - // by apply a new UUID for all the fresh disks. - for index, format := range formatConfigs { - if format == nil { + // Reorder the disks based on the JBOD order. + orderedDisks, err := reorderDisks(storageDisks, formatConfigs) + if err != nil { + return err + } + + // From ordered disks fill the UUID position. + for index, disk := range orderedDisks { + if disk == nil { newJBOD[index] = getUUID() } } + + // Collect new format configs. + var newFormatConfigs = make([]*formatConfigV1, len(orderedDisks)) + // Collect new format configs that need to be written. + for index := range orderedDisks { + // New configs are generated since we are going + // to re-populate across all disks. + config := &formatConfigV1{ + Version: referenceConfig.Version, + Format: referenceConfig.Format, + XL: &xlFormat{ + Version: referenceConfig.XL.Version, + Disk: newJBOD[index], + JBOD: newJBOD, + }, + } + newFormatConfigs[index] = config + } + + // Fill in the missing disk back from format configs. + // We need to make sure we have kept the previous order + // and allowed fresh disks to be arranged anywhere. + // Following block facilitates to put fresh disks. for index, format := range formatConfigs { + // Format is missing so we go through ordered disks. if format == nil { - config := &formatConfigV1{ - Version: referenceConfig.Version, - Format: referenceConfig.Format, - XL: &xlFormat{ - Version: referenceConfig.XL.Version, - Disk: newJBOD[index], - JBOD: newJBOD, - }, + // At this point when disk is missing the fresh disk + // in the stack get it back from storageDisks. + for oIndex, disk := range orderedDisks { + if disk == nil { + orderedDisks[oIndex] = storageDisks[index] + break + } } - newFormatConfigs[index] = config - continue } - newFormatConfigs[index] = format - newFormatConfigs[index].XL.JBOD = newJBOD - newFormatConfigs[index].XL.Disk = newJBOD[index] } - // Save new `format.json` across all disks. - return saveFormatXL(storageDisks, newFormatConfigs) + + // Save new `format.json` across all disks, in JBOD order. + return saveFormatXL(orderedDisks, newFormatConfigs) } // loadFormatXL - loads XL `format.json` and returns back properly diff --git a/xl-v1.go b/xl-v1.go index fb86ce232..745c280d9 100644 --- a/xl-v1.go +++ b/xl-v1.go @@ -152,7 +152,7 @@ func newXLObjects(disks, ignoredDisks []string) (ObjectLayer, error) { } case errSomeDiskUnformatted: // All drives online but some report missing format.json. - if err := healFormatXL(storageDisks); err != nil { + if err := healFormatXLFreshDisks(storageDisks); err != nil { // There was an unexpected unrecoverable error during healing. return nil, fmt.Errorf("Unable to heal backend %s", err) }