diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index de3f27493..0104d1086 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -117,7 +117,7 @@ func initTestXLObjLayer(ctx context.Context) (ObjectLayer, []string, error) { } globalPolicySys = NewPolicySys() - objLayer, err := newXLSets(ctx, endpoints, storageDisks, format, 1, 16) + objLayer, err := newXLSets(ctx, endpoints, storageDisks, format) if err != nil { return nil, nil, err } diff --git a/cmd/endpoint-ellipses.go b/cmd/endpoint-ellipses.go index a13900a81..b7a9f2486 100644 --- a/cmd/endpoint-ellipses.go +++ b/cmd/endpoint-ellipses.go @@ -18,6 +18,7 @@ package cmd import ( "fmt" + "sort" "strconv" "strings" @@ -63,11 +64,72 @@ var isValidSetSize = func(count uint64) bool { return (count >= setSizes[0] && count <= setSizes[len(setSizes)-1]) } +func commonSetDriveCount(divisibleSize uint64, setCounts []uint64) (setSize uint64) { + // prefers setCounts to be sorted for optimal behavior. + if divisibleSize < setCounts[len(setCounts)-1] { + return divisibleSize + } + + // Figure out largest value of total_drives_in_erasure_set which results + // in least number of total_drives/total_drives_erasure_set ratio. + prevD := divisibleSize / setCounts[0] + for _, cnt := range setCounts { + if divisibleSize%cnt == 0 { + d := divisibleSize / cnt + if d <= prevD { + prevD = d + setSize = cnt + } + } + } + return setSize +} + +// possibleSetCountsWithSymmetry returns symmetrical setCounts based on the +// input argument patterns, the symmetry calculation is to ensure that +// we also use uniform number of drives common across all ellipses patterns. +func possibleSetCountsWithSymmetry(setCounts []uint64, argPatterns []ellipses.ArgPattern) []uint64 { + var newSetCounts = make(map[uint64]struct{}) + for _, ss := range setCounts { + var symmetry bool + for _, argPattern := range argPatterns { + for _, p := range argPattern { + if uint64(len(p.Seq)) > ss { + symmetry = uint64(len(p.Seq))%ss == 0 + } else { + symmetry = ss%uint64(len(p.Seq)) == 0 + } + } + } + // With no arg patterns, it is expected that user knows + // the right symmetry, so either ellipses patterns are + // provided (recommended) or no ellipses patterns. + if _, ok := newSetCounts[ss]; !ok && (symmetry || argPatterns == nil) { + newSetCounts[ss] = struct{}{} + } + } + + setCounts = []uint64{} + for setCount := range newSetCounts { + setCounts = append(setCounts, setCount) + } + + // Not necessarily needed but it ensures to the readers + // eyes that we prefer a sorted setCount slice for the + // subsequent function to figure out the right common + // divisor, it avoids loops. + sort.Slice(setCounts, func(i, j int) bool { + return setCounts[i] < setCounts[j] + }) + + return setCounts +} + // getSetIndexes returns list of indexes which provides the set size // on each index, this function also determines the final set size // The final set size has the affinity towards choosing smaller // indexes (total sets) -func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint64) (setIndexes [][]uint64, err error) { +func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint64, argPatterns []ellipses.ArgPattern) (setIndexes [][]uint64, err error) { if len(totalSizes) == 0 || len(args) == 0 { return nil, errInvalidArgument } @@ -81,24 +143,7 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6 } } - var setSize uint64 - commonSize := getDivisibleSize(totalSizes) - if commonSize > setSizes[len(setSizes)-1] { - prevD := commonSize / setSizes[0] - for _, i := range setSizes { - if commonSize%i == 0 { - d := commonSize / i - if d <= prevD { - prevD = d - setSize = i - } - } - } - } else { - setSize = commonSize - } - possibleSetCounts := func(setSize uint64) (ss []uint64) { for _, s := range setSizes { if setSize%s == 0 { @@ -114,11 +159,11 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6 return nil, config.ErrInvalidNumberOfErasureEndpoints(nil).Msg(msg) } + var setSize uint64 + // Custom set drive count allows to override automatic distribution. + // only meant if you want to further optimize drive distribution. if customSetDriveCount > 0 { msg := fmt.Sprintf("Invalid set drive count. Acceptable values for %d number drives are %d", commonSize, setCounts) - if customSetDriveCount > setSize { - return nil, config.ErrInvalidErasureSetSize(nil).Msg(msg) - } var found bool for _, ss := range setCounts { if ss == customSetDriveCount { @@ -128,7 +173,16 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6 if !found { return nil, config.ErrInvalidErasureSetSize(nil).Msg(msg) } + + // No automatic symmetry calculation expected, user is on their own setSize = customSetDriveCount + globalCustomErasureDriveCount = true + } else { + // Returns possible set counts with symmetry. + setCounts = possibleSetCountsWithSymmetry(setCounts, argPatterns) + + // Final set size with all the symmetry accounted for. + setSize = commonSetDriveCount(commonSize, setCounts) } // Check whether setSize is with the supported range. @@ -202,7 +256,7 @@ func parseEndpointSet(customSetDriveCount uint64, args ...string) (ep endpointSe argPatterns[i] = patterns } - ep.setIndexes, err = getSetIndexes(args, getTotalSizes(argPatterns), customSetDriveCount) + ep.setIndexes, err = getSetIndexes(args, getTotalSizes(argPatterns), customSetDriveCount, argPatterns) if err != nil { return endpointSet{}, config.ErrInvalidErasureEndpoints(nil).Msg(err.Error()) } @@ -224,7 +278,7 @@ func GetAllSets(customSetDriveCount uint64, args ...string) ([][]string, error) // Check if we have more one args. if len(args) > 1 { var err error - setIndexes, err = getSetIndexes(args, []uint64{uint64(len(args))}, customSetDriveCount) + setIndexes, err = getSetIndexes(args, []uint64{uint64(len(args))}, customSetDriveCount, nil) if err != nil { return nil, err } @@ -258,6 +312,15 @@ func GetAllSets(customSetDriveCount uint64, args ...string) ([][]string, error) return setArgs, nil } +// Override set drive count for manual distribution. +const ( + EnvErasureSetDriveCount = "MINIO_ERASURE_SET_DRIVE_COUNT" +) + +var ( + globalCustomErasureDriveCount = false +) + // CreateServerEndpoints - validates and creates new endpoints from input args, supports // both ellipses and without ellipses transparently. func createServerEndpoints(serverAddr string, args ...string) ( @@ -268,7 +331,7 @@ func createServerEndpoints(serverAddr string, args ...string) ( return nil, -1, -1, errInvalidArgument } - if v := env.Get("MINIO_ERASURE_SET_DRIVE_COUNT", ""); v != "" { + if v := env.Get(EnvErasureSetDriveCount, ""); v != "" { setDriveCount, err = strconv.Atoi(v) if err != nil { return nil, -1, -1, config.ErrInvalidErasureSetSize(err) diff --git a/cmd/endpoint-ellipses_test.go b/cmd/endpoint-ellipses_test.go index 65150c01e..f73d776a4 100644 --- a/cmd/endpoint-ellipses_test.go +++ b/cmd/endpoint-ellipses_test.go @@ -104,11 +104,18 @@ func TestGetSetIndexesEnvOverride(t *testing.T) { 8, true, }, + { + []string{"http://host{1...2}/data{1...180}"}, + []uint64{360}, + [][]uint64{{15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15}}, + 15, + true, + }, { []string{"http://host{1...12}/data{1...12}"}, []uint64{144}, - [][]uint64{{16, 16, 16, 16, 16, 16, 16, 16, 16}}, - 16, + [][]uint64{{12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12}}, + 12, true, }, { @@ -160,7 +167,16 @@ func TestGetSetIndexesEnvOverride(t *testing.T) { for _, testCase := range testCases { testCase := testCase t.Run("", func(t *testing.T) { - gotIndexes, err := getSetIndexes(testCase.args, testCase.totalSizes, testCase.envOverride) + var argPatterns = make([]ellipses.ArgPattern, len(testCase.args)) + for i, arg := range testCase.args { + patterns, err := ellipses.FindEllipsesPatterns(arg) + if err != nil { + t.Fatalf("Unexpected failure %s", err) + } + argPatterns[i] = patterns + } + + gotIndexes, err := getSetIndexes(testCase.args, testCase.totalSizes, testCase.envOverride, argPatterns) if err != nil && testCase.success { t.Errorf("Expected success but failed instead %s", err) } @@ -202,6 +218,24 @@ func TestGetSetIndexes(t *testing.T) { [][]uint64{{9, 9, 9}}, true, }, + { + []string{"http://host{1...3}/data{1...180}"}, + []uint64{540}, + [][]uint64{{15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15, 15}}, + true, + }, + { + []string{"http://host{1...2}.rack{1...4}/data{1...180}"}, + []uint64{1440}, + [][]uint64{{16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16}}, + true, + }, + { + []string{"http://host{1...2}/data{1...180}"}, + []uint64{360}, + [][]uint64{{12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12, 12}}, + true, + }, { []string{"data/controller1/export{1...4}, data/controller2/export{1...8}, data/controller3/export{1...12}"}, []uint64{4, 8, 12}, @@ -243,7 +277,15 @@ func TestGetSetIndexes(t *testing.T) { for _, testCase := range testCases { testCase := testCase t.Run("", func(t *testing.T) { - gotIndexes, err := getSetIndexes(testCase.args, testCase.totalSizes, 0) + var argPatterns = make([]ellipses.ArgPattern, len(testCase.args)) + for i, arg := range testCase.args { + patterns, err := ellipses.FindEllipsesPatterns(arg) + if err != nil { + t.Fatalf("Unexpected failure %s", err) + } + argPatterns[i] = patterns + } + gotIndexes, err := getSetIndexes(testCase.args, testCase.totalSizes, 0, argPatterns) if err != nil && testCase.success { t.Errorf("Expected success but failed instead %s", err) } diff --git a/cmd/format-xl.go b/cmd/format-xl.go index c5f3408fd..3d0da44f4 100644 --- a/cmd/format-xl.go +++ b/cmd/format-xl.go @@ -456,7 +456,10 @@ func checkFormatXLValues(formats []*formatXLV3, drivesPerSet int) error { return fmt.Errorf("%s disk is already being used in another erasure deployment. (Number of disks specified: %d but the number of disks found in the %s disk's format.json: %d)", humanize.Ordinal(i+1), len(formats), humanize.Ordinal(i+1), len(formatXL.XL.Sets)*len(formatXL.XL.Sets[0])) } - if len(formatXL.XL.Sets[0]) != drivesPerSet { + // Only if custom erasure drive count is set, + // we should fail here other proceed to honor what + // is present on the disk. + if globalCustomErasureDriveCount && len(formatXL.XL.Sets[0]) != drivesPerSet { return fmt.Errorf("%s disk is already formatted with %d drives per erasure set. This cannot be changed to %d, please revert your MINIO_ERASURE_SET_DRIVE_COUNT setting", humanize.Ordinal(i+1), len(formatXL.XL.Sets[0]), drivesPerSet) } } diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 931c50ab7..269013bc4 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -182,7 +182,7 @@ func prepareXLSets32(ctx context.Context) (ObjectLayer, []string, error) { return nil, nil, err } - objAPI, err := newXLSets(ctx, endpoints, storageDisks, format, 2, 16) + objAPI, err := newXLSets(ctx, endpoints, storageDisks, format) if err != nil { return nil, nil, err } diff --git a/cmd/xl-sets.go b/cmd/xl-sets.go index 257679ab4..e955198b9 100644 --- a/cmd/xl-sets.go +++ b/cmd/xl-sets.go @@ -293,7 +293,7 @@ func (s *xlSets) GetDisks(setIndex int) func() []StorageAPI { const defaultMonitorConnectEndpointInterval = time.Second * 10 // Set to 10 secs. // Initialize new set of erasure coded sets. -func newXLSets(ctx context.Context, endpoints Endpoints, storageDisks []StorageAPI, format *formatXLV3, setCount int, drivesPerSet int) (*xlSets, error) { +func newXLSets(ctx context.Context, endpoints Endpoints, storageDisks []StorageAPI, format *formatXLV3) (*xlSets, error) { endpointStrings := make([]string, len(endpoints)) for i, endpoint := range endpoints { if endpoint.IsLocal { @@ -303,6 +303,9 @@ func newXLSets(ctx context.Context, endpoints Endpoints, storageDisks []StorageA } } + setCount := len(format.XL.Sets) + drivesPerSet := len(format.XL.Sets[0]) + // Initialize the XL sets instance. s := &xlSets{ sets: make([]*xlObjects, setCount), diff --git a/cmd/xl-sets_test.go b/cmd/xl-sets_test.go index 9567722c2..91d2f9522 100644 --- a/cmd/xl-sets_test.go +++ b/cmd/xl-sets_test.go @@ -96,7 +96,7 @@ func TestNewXLSets(t *testing.T) { t.Fatalf("Unable to format disks for erasure, %s", err) } - if _, err := newXLSets(ctx, endpoints, storageDisks, format, 1, 16); err != nil { + if _, err := newXLSets(ctx, endpoints, storageDisks, format); err != nil { t.Fatalf("Unable to initialize erasure") } } diff --git a/cmd/xl-zones.go b/cmd/xl-zones.go index 30f964303..e896df128 100644 --- a/cmd/xl-zones.go +++ b/cmd/xl-zones.go @@ -82,7 +82,7 @@ func newXLZones(ctx context.Context, endpointZones EndpointZones) (ObjectLayer, if deploymentID == "" { deploymentID = formats[i].ID } - z.zones[i], err = newXLSets(ctx, ep.Endpoints, storageDisks[i], formats[i], ep.SetCount, ep.DrivesPerSet) + z.zones[i], err = newXLSets(ctx, ep.Endpoints, storageDisks[i], formats[i]) if err != nil { return nil, err }