From 4669d19f2a854d90b4993471f77ce704507018a0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 16 Jun 2021 14:26:26 -0700 Subject: [PATCH] fix: simplify diskMap usage to keep certain checks predictable (#12519) Bonus: also make sure that we Sanitize() the drives only during startup of the server, but not during disk reconnects. --- cmd/erasure-sets.go | 14 +++++--------- cmd/prepare-storage.go | 15 +++++++++++---- cmd/storage-rest-common.go | 2 +- cmd/xl-storage.go | 18 ++++++++++-------- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/cmd/erasure-sets.go b/cmd/erasure-sets.go index d7c835e05..caff485e3 100644 --- a/cmd/erasure-sets.go +++ b/cmd/erasure-sets.go @@ -102,7 +102,7 @@ type erasureSets struct { } // Return false if endpoint is not connected or has been reconnected after last check -func isEndpointConnectionStable(diskMap map[string]StorageAPI, endpoint string, lastCheck time.Time) bool { +func isEndpointConnectionStable(diskMap map[Endpoint]StorageAPI, endpoint Endpoint, lastCheck time.Time) bool { disk := diskMap[endpoint] if disk == nil { return false @@ -116,8 +116,8 @@ func isEndpointConnectionStable(diskMap map[string]StorageAPI, endpoint string, return true } -func (s *erasureSets) getDiskMap() map[string]StorageAPI { - diskMap := make(map[string]StorageAPI) +func (s *erasureSets) getDiskMap() map[Endpoint]StorageAPI { + diskMap := make(map[Endpoint]StorageAPI) s.erasureDisksMu.RLock() defer s.erasureDisksMu.RUnlock() @@ -131,7 +131,7 @@ func (s *erasureSets) getDiskMap() map[string]StorageAPI { if !disk.IsOnline() { continue } - diskMap[disk.String()] = disk + diskMap[disk.Endpoint()] = disk } } return diskMap @@ -213,11 +213,7 @@ func (s *erasureSets) connectDisks() { var setsJustConnected = make([]bool, s.setCount) diskMap := s.getDiskMap() for _, endpoint := range s.endpoints { - diskPath := endpoint.String() - if endpoint.IsLocal { - diskPath = endpoint.Path - } - if isEndpointConnectionStable(diskMap, diskPath, s.lastConnectDisksOpTime) { + if isEndpointConnectionStable(diskMap, endpoint, s.lastConnectDisksOpTime) { continue } wg.Add(1) diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go index 9c00b8b5b..047c399e1 100644 --- a/cmd/prepare-storage.go +++ b/cmd/prepare-storage.go @@ -72,16 +72,16 @@ var printEndpointError = func() func(Endpoint, error, bool) { // Cleans up tmp directory of the local disk. func formatErasureCleanupTmp(diskPath string) error { // Need to move temporary objects left behind from previous run of minio - // server to a unique directory under `minioMetaTmpBucket-old` to clean + // server to a unique directory under `minioMetaTmpBucket/.trash` to clean // up `minioMetaTmpBucket` for the current run. // - // /disk1/.minio.sys/tmp-old/ + // /disk1/.minio.sys/tmp/.trash/uuid // |__ 33a58b40-aecc-4c9f-a22f-ff17bfa33b62 // |__ e870a2c1-d09c-450c-a69c-6eaa54a89b3e // // In this example, `33a58b40-aecc-4c9f-a22f-ff17bfa33b62` directory contains // temporary objects from one of the previous runs of minio server. - tmpOld := pathJoin(diskPath, minioMetaTmpBucket+"-old", mustGetUUID()) + tmpOld := pathJoin(diskPath, minioMetaTmpDeletedBucket, mustGetUUID()) if err := renameAll(pathJoin(diskPath, minioMetaTmpBucket), tmpOld); err != nil && err != errFileNotFound { logger.LogIf(GlobalContext, fmt.Errorf("unable to rename (%s -> %s) %w, drive may be faulty please investigate", @@ -94,7 +94,7 @@ func formatErasureCleanupTmp(diskPath string) error { renameAllBucketMetacache(diskPath) // Removal of tmp-old folder is backgrounded completely. - go removeAll(pathJoin(diskPath, minioMetaTmpBucket+"-old")) + go removeAll(tmpOld) if err := mkdirAll(pathJoin(diskPath, minioMetaTmpBucket), 0777); err != nil { logger.LogIf(GlobalContext, fmt.Errorf("unable to create (%s) %w, drive may be faulty please investigate", @@ -178,6 +178,13 @@ func connectLoadInitFormats(retryCount int, firstDisk bool, endpoints Endpoints, } }(storageDisks) + // Sanitize all local disks during server startup. + for _, disk := range storageDisks { + if disk != nil && disk.IsLocal() { + disk.(*xlStorageDiskIDCheck).storage.(*xlStorage).Sanitize() + } + } + for i, err := range errs { if err != nil { if err == errDiskNotFound && retryCount >= 5 { diff --git a/cmd/storage-rest-common.go b/cmd/storage-rest-common.go index 62f8ee7db..f05a556a9 100644 --- a/cmd/storage-rest-common.go +++ b/cmd/storage-rest-common.go @@ -18,7 +18,7 @@ package cmd const ( - storageRESTVersion = "v36" // Changes to FileInfo for tier-journal + storageRESTVersion = "v37" // cleanup behavior change at storage layer. storageRESTVersionPrefix = SlashSeparator + storageRESTVersion storageRESTPrefix = minioReservedBucketPath + "/storage" ) diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 73b04d705..b6316dbcc 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -233,6 +233,16 @@ func newLocalXLStorage(path string) (*xlStorage, error) { }) } +// Sanitize - sanitizes the `format.json`, cleanup tmp. +// all other future cleanups should be added here. +func (s *xlStorage) Sanitize() error { + if err := formatErasureMigrate(s.diskPath); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + + return formatErasureCleanupTmp(s.diskPath) +} + // Initialize a new storage disk. func newXLStorage(ep Endpoint) (*xlStorage, error) { path := ep.Path @@ -302,14 +312,6 @@ func newXLStorage(ep Endpoint) (*xlStorage, error) { w.Close() Remove(filePath) - if err := formatErasureMigrate(p.diskPath); err != nil && !errors.Is(err, os.ErrNotExist) { - return p, err - } - - if err := formatErasureCleanupTmp(p.diskPath); err != nil { - return p, err - } - // Success. return p, nil }