Skip to content

Commit

Permalink
xl: Fix counting offline disks in StorageInfo (minio#9082)
Browse files Browse the repository at this point in the history
Recent modification in the code led to incorrect calculation
of offline disks.

This commit saves the endpoint list in a xlObjects then we know
the name of each disk.
  • Loading branch information
kannappanr authored Mar 5, 2020
1 parent c7ca791 commit 07a7f32
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 16 deletions.
6 changes: 6 additions & 0 deletions cmd/xl-sets.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,16 @@ func newXLSets(endpoints Endpoints, format *formatXLV3, setCount int, drivesPerS
s.xlDisks[i] = make([]StorageAPI, drivesPerSet)
s.xlLockers[i] = make([]dsync.NetLocker, drivesPerSet)

var endpoints Endpoints
for j := 0; j < drivesPerSet; j++ {
endpoints = append(endpoints, s.endpoints[i*s.drivesPerSet+j])
}

// Initialize xl objects for a given set.
s.sets[i] = &xlObjects{
getDisks: s.GetDisks(i),
getLockers: s.GetLockers(i),
endpoints: endpoints,
nsMutex: mutex,
bp: bp,
mrfUploadCh: make(chan partialUpload, 10000),
Expand Down
28 changes: 12 additions & 16 deletions cmd/xl-v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ type xlObjects struct {
// getLockers returns list of remote and local lockers.
getLockers func() []dsync.NetLocker

endpoints Endpoints

// Locker mutex map.
nsMutex *nsLockMap

Expand Down Expand Up @@ -87,7 +89,7 @@ func (d byDiskTotal) Less(i, j int) bool {
}

// getDisksInfo - fetch disks info across all other storage API.
func getDisksInfo(disks []StorageAPI) (disksInfo []DiskInfo, onlineDisks, offlineDisks madmin.BackendDisks) {
func getDisksInfo(disks []StorageAPI, endpoints Endpoints) (disksInfo []DiskInfo, onlineDisks, offlineDisks madmin.BackendDisks) {
disksInfo = make([]DiskInfo, len(disks))

g := errgroup.WithNErrs(len(disks))
Expand Down Expand Up @@ -115,24 +117,16 @@ func getDisksInfo(disks []StorageAPI) (disksInfo []DiskInfo, onlineDisks, offlin
onlineDisks = make(madmin.BackendDisks)
offlineDisks = make(madmin.BackendDisks)

localNodeAddr := GetLocalPeer(globalEndpoints)

// Wait for the routines.
for i, diskInfoErr := range g.Wait() {
if disks[i] == nil {
continue
}
peerAddr := disks[i].Hostname()
if peerAddr == "" {
peerAddr = localNodeAddr
}
peerAddr := endpoints[i].Host
if _, ok := offlineDisks[peerAddr]; !ok {
offlineDisks[peerAddr] = 0
}
if _, ok := onlineDisks[peerAddr]; !ok {
onlineDisks[peerAddr] = 0
}
if diskInfoErr != nil {
if disks[i] == nil || diskInfoErr != nil {
offlineDisks[peerAddr]++
continue
}
Expand All @@ -144,8 +138,8 @@ func getDisksInfo(disks []StorageAPI) (disksInfo []DiskInfo, onlineDisks, offlin
}

// Get an aggregated storage info across all disks.
func getStorageInfo(disks []StorageAPI) StorageInfo {
disksInfo, onlineDisks, offlineDisks := getDisksInfo(disks)
func getStorageInfo(disks []StorageAPI, endpoints Endpoints) StorageInfo {
disksInfo, onlineDisks, offlineDisks := getDisksInfo(disks, endpoints)

// Sort so that the first element is the smallest.
sort.Sort(byDiskTotal(disksInfo))
Expand Down Expand Up @@ -179,18 +173,20 @@ func getStorageInfo(disks []StorageAPI) StorageInfo {

// StorageInfo - returns underlying storage statistics.
func (xl xlObjects) StorageInfo(ctx context.Context, local bool) StorageInfo {
var endpoints = xl.endpoints
var disks []StorageAPI

if !local {
disks = xl.getDisks()
} else {
for _, d := range xl.getDisks() {
if d != nil && d.Hostname() == "" {
for i, d := range xl.getDisks() {
if endpoints[i].IsLocal {
// Append this local disk since local flag is true
disks = append(disks, d)
}
}
}
return getStorageInfo(disks)
return getStorageInfo(disks, endpoints)
}

// GetMetrics - is not implemented and shouldn't be called.
Expand Down

0 comments on commit 07a7f32

Please sign in to comment.