From 756d6aa7296cd5842d141af30ffdf1773ed067b9 Mon Sep 17 00:00:00 2001 From: Anis Eleuch Date: Thu, 20 Jul 2023 15:48:21 +0100 Subject: [PATCH] fix: report correct pool/set/disk indexes for offline disks (#17695) --- cmd/endpoint-ellipses.go | 2 +- cmd/endpoint.go | 50 +++++++++++++++++++++++++++++--------- cmd/endpoint_test.go | 6 ++--- cmd/erasure-sets_test.go | 2 +- cmd/erasure.go | 45 +++++++++++++++++----------------- cmd/format-erasure_test.go | 5 ++-- cmd/test-utils_test.go | 14 +++++++---- 7 files changed, 78 insertions(+), 46 deletions(-) diff --git a/cmd/endpoint-ellipses.go b/cmd/endpoint-ellipses.go index 417efe1b4..4f1e5549e 100644 --- a/cmd/endpoint-ellipses.go +++ b/cmd/endpoint-ellipses.go @@ -360,7 +360,7 @@ func createServerEndpoints(serverAddr string, args ...string) ( return nil, -1, err } for i := range endpointList { - endpointList[i].SetPool(0) + endpointList[i].SetPoolIndex(0) } endpointServerPools = append(endpointServerPools, PoolEndpoints{ Legacy: true, diff --git a/cmd/endpoint.go b/cmd/endpoint.go index 091777c2c..305e81b6c 100644 --- a/cmd/endpoint.go +++ b/cmd/endpoint.go @@ -69,8 +69,9 @@ type Node struct { // Endpoint - any type of endpoint. type Endpoint struct { *url.URL - Pool int IsLocal bool + + PoolIdx, SetIdx, DiskIdx int } func (endpoint Endpoint) String() string { @@ -106,9 +107,19 @@ func (endpoint *Endpoint) UpdateIsLocal() (err error) { return nil } -// SetPool sets a specific pool number to this node -func (endpoint *Endpoint) SetPool(i int) { - endpoint.Pool = i +// SetPoolIndex sets a specific pool number to this node +func (endpoint *Endpoint) SetPoolIndex(i int) { + endpoint.PoolIdx = i +} + +// SetSetIndex sets a specific set number to this node +func (endpoint *Endpoint) SetSetIndex(i int) { + endpoint.SetIdx = i +} + +// SetDiskIndex sets a specific disk number to this node +func (endpoint *Endpoint) SetDiskIndex(i int) { + endpoint.DiskIdx = i } // NewEndpoint - returns new endpoint based on given arguments. @@ -205,6 +216,9 @@ func NewEndpoint(arg string) (ep Endpoint, e error) { return Endpoint{ URL: u, IsLocal: isLocal, + PoolIdx: -1, + SetIdx: -1, + DiskIdx: -1, }, nil } @@ -236,8 +250,8 @@ func (l EndpointServerPools) GetNodes() (nodes []Node) { Host: ep.Host, } } - if !slices.Contains(node.Pools, ep.Pool) { - node.Pools = append(node.Pools, ep.Pool) + if !slices.Contains(node.Pools, ep.PoolIdx) { + node.Pools = append(node.Pools, ep.PoolIdx) } nodesMap[ep.Host] = node } @@ -811,7 +825,9 @@ func CreatePoolEndpoints(serverAddr string, poolArgs ...[][]string) ([]Endpoints return nil, setupType, config.ErrInvalidEndpoint(nil).Msg("use path style endpoint for single node setup") } - endpoint.SetPool(0) + endpoint.SetPoolIndex(0) + endpoint.SetSetIndex(0) + endpoint.SetDiskIndex(0) var endpoints Endpoints endpoints = append(endpoints, endpoint) @@ -828,7 +844,7 @@ func CreatePoolEndpoints(serverAddr string, poolArgs ...[][]string) ([]Endpoints for poolIdx, args := range poolArgs { var endpoints Endpoints - for _, iargs := range args { + for setIdx, iargs := range args { // Convert args to endpoints eps, err := NewEndpoints(iargs...) if err != nil { @@ -840,8 +856,10 @@ func CreatePoolEndpoints(serverAddr string, poolArgs ...[][]string) ([]Endpoints return nil, setupType, config.ErrInvalidErasureEndpoints(nil).Msg(err.Error()) } - for i := range eps { - eps[i].SetPool(poolIdx) + for diskIdx := range eps { + eps[diskIdx].SetPoolIndex(poolIdx) + eps[diskIdx].SetSetIndex(setIdx) + eps[diskIdx].SetDiskIndex(diskIdx) } endpoints = append(endpoints, eps...) @@ -1022,6 +1040,11 @@ func CreateEndpoints(serverAddr string, args ...[]string) (Endpoints, SetupType, if endpoint.Type() != PathEndpointType { return endpoints, setupType, config.ErrInvalidEndpoint(nil).Msg("use path style endpoint for single node setup") } + + endpoint.SetPoolIndex(0) + endpoint.SetSetIndex(0) + endpoint.SetDiskIndex(0) + endpoints = append(endpoints, endpoint) setupType = ErasureSDSetupType @@ -1033,7 +1056,7 @@ func CreateEndpoints(serverAddr string, args ...[]string) (Endpoints, SetupType, return endpoints, setupType, nil } - for _, iargs := range args { + for setIdx, iargs := range args { // Convert args to endpoints eps, err := NewEndpoints(iargs...) if err != nil { @@ -1045,6 +1068,11 @@ func CreateEndpoints(serverAddr string, args ...[]string) (Endpoints, SetupType, return endpoints, setupType, config.ErrInvalidErasureEndpoints(nil).Msg(err.Error()) } + for diskIdx := range eps { + eps[diskIdx].SetSetIndex(setIdx) + eps[diskIdx].SetDiskIndex(diskIdx) + } + endpoints = append(endpoints, eps...) } diff --git a/cmd/endpoint_test.go b/cmd/endpoint_test.go index e323e1e41..b2bed1115 100644 --- a/cmd/endpoint_test.go +++ b/cmd/endpoint_test.go @@ -37,9 +37,9 @@ func TestNewEndpoint(t *testing.T) { expectedType EndpointType expectedErr error }{ - {"/foo", Endpoint{URL: &url.URL{Path: rootSlashFoo}, IsLocal: true}, PathEndpointType, nil}, - {"https://example.org/path", Endpoint{URL: u2, IsLocal: false}, URLEndpointType, nil}, - {"http://192.168.253.200/path", Endpoint{URL: u4, IsLocal: false}, URLEndpointType, nil}, + {"/foo", Endpoint{&url.URL{Path: rootSlashFoo}, true, -1, -1, -1}, PathEndpointType, nil}, + {"https://example.org/path", Endpoint{u2, false, -1, -1, -1}, URLEndpointType, nil}, + {"http://192.168.253.200/path", Endpoint{u4, false, -1, -1, -1}, URLEndpointType, nil}, {"", Endpoint{}, -1, fmt.Errorf("empty or root endpoint is not supported")}, {SlashSeparator, Endpoint{}, -1, fmt.Errorf("empty or root endpoint is not supported")}, {`\`, Endpoint{}, -1, fmt.Errorf("empty or root endpoint is not supported")}, diff --git a/cmd/erasure-sets_test.go b/cmd/erasure-sets_test.go index cb0c15fa5..1e411d6c9 100644 --- a/cmd/erasure-sets_test.go +++ b/cmd/erasure-sets_test.go @@ -173,7 +173,7 @@ func TestNewErasureSets(t *testing.T) { defer os.RemoveAll(disk) } - endpoints := mustGetNewEndpoints(0, erasureDisks...) + endpoints := mustGetNewEndpoints(0, 16, erasureDisks...) _, _, err := waitForFormatErasure(true, endpoints, 1, 0, 16, "", "") if err != errInvalidArgument { t.Fatalf("Expecting error, got %s", err) diff --git a/cmd/erasure.go b/cmd/erasure.go index 900c50516..d012b259c 100644 --- a/cmd/erasure.go +++ b/cmd/erasure.go @@ -174,33 +174,32 @@ func getDisksInfo(disks []StorageAPI, endpoints []Endpoint) (disksInfo []madmin. for index := range disks { index := index g.Go(func() error { - diskEndpoint := endpoints[index].String() + di := madmin.Disk{ + Endpoint: endpoints[index].String(), + PoolIndex: endpoints[index].PoolIdx, + SetIndex: endpoints[index].SetIdx, + DiskIndex: endpoints[index].DiskIdx, + } if disks[index] == OfflineDisk { - logger.LogOnceIf(GlobalContext, fmt.Errorf("%s: %s", errDiskNotFound, endpoints[index]), "get-disks-info-offline-"+diskEndpoint) - disksInfo[index] = madmin.Disk{ - State: diskErrToDriveState(errDiskNotFound), - Endpoint: diskEndpoint, - } + logger.LogOnceIf(GlobalContext, fmt.Errorf("%s: %s", errDiskNotFound, endpoints[index]), "get-disks-info-offline-"+di.Endpoint) + di.State = diskErrToDriveState(errDiskNotFound) + disksInfo[index] = di return nil } info, err := disks[index].DiskInfo(context.TODO()) - di := madmin.Disk{ - Endpoint: diskEndpoint, - DrivePath: info.MountPath, - TotalSpace: info.Total, - UsedSpace: info.Used, - AvailableSpace: info.Free, - UUID: info.ID, - Major: info.Major, - Minor: info.Minor, - RootDisk: info.RootDisk, - Healing: info.Healing, - Scanning: info.Scanning, - State: diskErrToDriveState(err), - FreeInodes: info.FreeInodes, - UsedInodes: info.UsedInodes, - } - di.PoolIndex, di.SetIndex, di.DiskIndex = disks[index].GetDiskLoc() + di.DrivePath = info.MountPath + di.TotalSpace = info.Total + di.UsedSpace = info.Used + di.AvailableSpace = info.Free + di.UUID = info.ID + di.Major = info.Major + di.Minor = info.Minor + di.RootDisk = info.RootDisk + di.Healing = info.Healing + di.Scanning = info.Scanning + di.State = diskErrToDriveState(err) + di.FreeInodes = info.FreeInodes + di.UsedInodes = info.UsedInodes if info.Healing { if hi := disks[index].Healing(); hi != nil { hd := hi.toHealingDisk() diff --git a/cmd/format-erasure_test.go b/cmd/format-erasure_test.go index 7aaf63e03..eb380f37c 100644 --- a/cmd/format-erasure_test.go +++ b/cmd/format-erasure_test.go @@ -36,7 +36,7 @@ func TestFixFormatV3(t *testing.T) { for _, erasureDir := range erasureDirs { defer os.RemoveAll(erasureDir) } - endpoints := mustGetNewEndpoints(0, erasureDirs...) + endpoints := mustGetNewEndpoints(0, 8, erasureDirs...) storageDisks, errs := initStorageDisksWithErrors(endpoints, false) for _, err := range errs { @@ -555,7 +555,8 @@ func benchmarkInitStorageDisksN(b *testing.B, nDisks int) { if err != nil { b.Fatal(err) } - endpoints := mustGetNewEndpoints(0, fsDirs...) + + endpoints := mustGetNewEndpoints(0, 16, fsDirs...) b.RunParallel(func(pb *testing.PB) { endpoints := endpoints for pb.Next() { diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 6c6c89314..916f403ad 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -2173,13 +2173,13 @@ func generateTLSCertKey(host string) ([]byte, []byte, error) { } func mustGetPoolEndpoints(poolIdx int, args ...string) EndpointServerPools { - endpoints := mustGetNewEndpoints(poolIdx, args...) drivesPerSet := len(args) setCount := 1 if len(args) >= 16 { drivesPerSet = 16 setCount = len(args) / 16 } + endpoints := mustGetNewEndpoints(poolIdx, drivesPerSet, args...) return []PoolEndpoints{{ SetCount: setCount, DrivesPerSet: drivesPerSet, @@ -2188,12 +2188,16 @@ func mustGetPoolEndpoints(poolIdx int, args ...string) EndpointServerPools { }} } -func mustGetNewEndpoints(poolIdx int, args ...string) (endpoints Endpoints) { +func mustGetNewEndpoints(poolIdx int, drivesPerSet int, args ...string) (endpoints Endpoints) { endpoints, err := NewEndpoints(args...) - for i := range endpoints { - endpoints[i].Pool = poolIdx + if err != nil { + panic(err) + } + for i := range endpoints { + endpoints[i].SetPoolIndex(poolIdx) + endpoints[i].SetSetIndex(i / drivesPerSet) + endpoints[i].SetDiskIndex(i % drivesPerSet) } - logger.FatalIf(err, "unable to create new endpoint list") return endpoints }