From f8e13fb00efb40860a7ccb21fc816adad0a90fc3 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 17 Oct 2016 14:31:33 -0700 Subject: [PATCH] server: Startup sequence should be more idempotent. (#2974) Fixes #2971 - honors ignore-disks option properly. Fixes #2969 - change the net.Dial to have a timeout of 3secs. --- cmd/lock-rpc-server_test.go | 2 +- cmd/net-rpc-client.go | 4 +++- cmd/object-common.go | 2 +- cmd/prepare-storage-msg.go | 9 +++++++ cmd/xl-v1.go | 34 +++++++++++++++++++------- cmd/xl-v1_test.go | 48 +++++++++++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 11 deletions(-) diff --git a/cmd/lock-rpc-server_test.go b/cmd/lock-rpc-server_test.go index 12814dd78..0215c3500 100644 --- a/cmd/lock-rpc-server_test.go +++ b/cmd/lock-rpc-server_test.go @@ -339,7 +339,7 @@ func TestLockRpcServerForceUnlock(t *testing.T) { Timestamp: timestamp, Node: "node", RPCPath: "rpc-path", - UID: "1234-5678", + UID: "1234-5678", } // First test that UID should be empty diff --git a/cmd/net-rpc-client.go b/cmd/net-rpc-client.go index fd7fe1710..ee971db4d 100644 --- a/cmd/net-rpc-client.go +++ b/cmd/net-rpc-client.go @@ -25,6 +25,7 @@ import ( "net/http" "net/rpc" "sync" + "time" ) // RPCClient is a wrapper type for rpc.Client which provides reconnect on first failure. @@ -78,7 +79,8 @@ func (rpcClient *RPCClient) dialRPCClient() (*rpc.Client, error) { if rpcClient.secureConn { conn, err = tls.Dial("tcp", rpcClient.node, &tls.Config{}) } else { - conn, err = net.Dial("tcp", rpcClient.node) + // Have a dial timeout with 3 secs. + conn, err = net.DialTimeout("tcp", rpcClient.node, 3*time.Second) } if err != nil { return nil, err diff --git a/cmd/object-common.go b/cmd/object-common.go index 2c076d668..41ee43027 100644 --- a/cmd/object-common.go +++ b/cmd/object-common.go @@ -194,7 +194,7 @@ func xlHouseKeeping(storageDisks []StorageAPI) error { err := cleanupDir(disk, minioMetaBucket, tmpMetaPrefix) if err != nil { switch errorCause(err) { - case errDiskNotFound, errVolumeNotFound: + case errDiskNotFound, errVolumeNotFound, errFileNotFound: default: errs[index] = err } diff --git a/cmd/prepare-storage-msg.go b/cmd/prepare-storage-msg.go index 5b4204732..1887d5158 100644 --- a/cmd/prepare-storage-msg.go +++ b/cmd/prepare-storage-msg.go @@ -65,6 +65,9 @@ func getHealMsg(firstEndpoint string, storageDisks []StorageAPI) string { msg = fmt.Sprintf(msg, creds.AccessKeyID, creds.SecretAccessKey, firstEndpoint) disksInfo, _, _ := getDisksInfo(storageDisks) for i, info := range disksInfo { + if storageDisks[i] == nil { + continue + } msg += fmt.Sprintf( "\n[%s] %s - %s %s", int2Str(i+1, len(storageDisks)), @@ -92,6 +95,9 @@ func getRegularMsg(storageDisks []StorageAPI) string { msg := colorBlue("\nInitializing data volume.") disksInfo, _, _ := getDisksInfo(storageDisks) for i, info := range disksInfo { + if storageDisks[i] == nil { + continue + } msg += fmt.Sprintf( "\n[%s] %s - %s %s", int2Str(i+1, len(storageDisks)), @@ -119,6 +125,9 @@ func getFormatMsg(storageDisks []StorageAPI) string { msg := colorBlue("\nInitializing data volume for the first time.") disksInfo, _, _ := getDisksInfo(storageDisks) for i, info := range disksInfo { + if storageDisks[i] == nil { + continue + } msg += fmt.Sprintf( "\n[%s] %s - %s %s", int2Str(i+1, len(storageDisks)), diff --git a/cmd/xl-v1.go b/cmd/xl-v1.go index ebfa36935..4020d36a1 100644 --- a/cmd/xl-v1.go +++ b/cmd/xl-v1.go @@ -170,7 +170,8 @@ func (d byDiskTotal) Less(i, j int) bool { // getDisksInfo - fetch disks info across all other storage API. func getDisksInfo(disks []StorageAPI) (disksInfo []disk.Info, onlineDisks int, offlineDisks int) { - for _, storageDisk := range disks { + disksInfo = make([]disk.Info, len(disks)) + for i, storageDisk := range disks { if storageDisk == nil { // Storage disk is empty, perhaps ignored disk or not available. offlineDisks++ @@ -185,32 +186,49 @@ func getDisksInfo(disks []StorageAPI) (disksInfo []disk.Info, onlineDisks int, o continue } onlineDisks++ - disksInfo = append(disksInfo, info) + disksInfo[i] = info } - // Sort so that the first element is the smallest. - sort.Sort(byDiskTotal(disksInfo)) - // Success. return disksInfo, onlineDisks, offlineDisks } +// returns sorted disksInfo slice which has only valid entries. +// i.e the entries where the total size of the disk is not stated +// as 0Bytes, this means that the disk is not online or ignored. +func sortValidDisksInfo(disksInfo []disk.Info) []disk.Info { + var validDisksInfo []disk.Info + for _, diskInfo := range disksInfo { + if diskInfo.Total == 0 { + continue + } + validDisksInfo = append(validDisksInfo, diskInfo) + } + sort.Sort(byDiskTotal(validDisksInfo)) + return validDisksInfo +} + // Get an aggregated storage info across all disks. func getStorageInfo(disks []StorageAPI) StorageInfo { disksInfo, onlineDisks, offlineDisks := getDisksInfo(disks) - if len(disksInfo) == 0 { + + // Sort so that the first element is the smallest. + validDisksInfo := sortValidDisksInfo(disksInfo) + if len(validDisksInfo) == 0 { return StorageInfo{ Total: -1, Free: -1, } } + // Return calculated storage info, choose the lowest Total and // Free as the total aggregated values. Total capacity is always // the multiple of smallest disk among the disk list. storageInfo := StorageInfo{ - Total: disksInfo[0].Total * int64(onlineDisks) / 2, - Free: disksInfo[0].Free * int64(onlineDisks) / 2, + Total: validDisksInfo[0].Total * int64(onlineDisks) / 2, + Free: validDisksInfo[0].Free * int64(onlineDisks) / 2, } + storageInfo.Backend.Type = XL storageInfo.Backend.OnlineDisks = onlineDisks storageInfo.Backend.OfflineDisks = offlineDisks diff --git a/cmd/xl-v1_test.go b/cmd/xl-v1_test.go index 9b15fceca..a1ff49fc7 100644 --- a/cmd/xl-v1_test.go +++ b/cmd/xl-v1_test.go @@ -19,7 +19,10 @@ package cmd import ( "os" "path/filepath" + "reflect" "testing" + + "github.com/minio/minio/pkg/disk" ) // TestStorageInfo - tests storage info. @@ -73,6 +76,51 @@ func TestStorageInfo(t *testing.T) { } } +// Sort valid disks info. +func TestSortingValidDisks(t *testing.T) { + testCases := []struct { + disksInfo []disk.Info + validDisksInfo []disk.Info + }{ + // One of the disks is offline. + { + disksInfo: []disk.Info{ + {Total: 150, Free: 10}, + {Total: 0, Free: 0}, + {Total: 200, Free: 10}, + {Total: 100, Free: 10}, + }, + validDisksInfo: []disk.Info{ + {Total: 100, Free: 10}, + {Total: 150, Free: 10}, + {Total: 200, Free: 10}, + }, + }, + // All disks are online. + { + disksInfo: []disk.Info{ + {Total: 150, Free: 10}, + {Total: 200, Free: 10}, + {Total: 100, Free: 10}, + {Total: 115, Free: 10}, + }, + validDisksInfo: []disk.Info{ + {Total: 100, Free: 10}, + {Total: 115, Free: 10}, + {Total: 150, Free: 10}, + {Total: 200, Free: 10}, + }, + }, + } + + for i, testCase := range testCases { + validDisksInfo := sortValidDisksInfo(testCase.disksInfo) + if !reflect.DeepEqual(validDisksInfo, testCase.validDisksInfo) { + t.Errorf("Test %d: Expected %#v, Got %#v", i+1, testCase.validDisksInfo, validDisksInfo) + } + } +} + // TestNewXL - tests initialization of all input disks // and constructs a valid `XL` object func TestNewXL(t *testing.T) {