From 7d50361ca98df0f33bbe675b15c87e228e8f5707 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Thu, 20 Oct 2016 08:29:48 +0530 Subject: [PATCH] Move housekeeping before object layer initialization (#3001) In a distributed setup that the server should not perform any operation on the storage layer after it is exported via RPC. e.g, cleaning up of temporary directories under .minio.sys/tmp may interfere with ongoing PUT objects being served by the distributed setup. --- cmd/fs-v1.go | 5 -- cmd/naughty-disk_test.go | 2 +- cmd/object-common.go | 91 +++++++++++++++-------------------- cmd/object-common_test.go | 99 +++++++++++++++++++++++++++++++++++++++ cmd/server-main.go | 3 ++ cmd/xl-v1.go | 5 -- 6 files changed, 142 insertions(+), 63 deletions(-) create mode 100644 cmd/object-common_test.go diff --git a/cmd/fs-v1.go b/cmd/fs-v1.go index d7ed218e6..b1a753454 100644 --- a/cmd/fs-v1.go +++ b/cmd/fs-v1.go @@ -52,11 +52,6 @@ func newFSObjects(storage StorageAPI) (ObjectLayer, error) { return nil, errInvalidArgument } - // Runs house keeping code, like creating minioMetaBucket, cleaning up tmp files etc. - if err := fsHouseKeeping(storage); err != nil { - return nil, err - } - // Load format and validate. _, err := loadFormatFS(storage) if err != nil { diff --git a/cmd/naughty-disk_test.go b/cmd/naughty-disk_test.go index 0118c37ce..34c0c68e0 100644 --- a/cmd/naughty-disk_test.go +++ b/cmd/naughty-disk_test.go @@ -44,7 +44,7 @@ func newNaughtyDisk(d *posix, errs map[int]error, defaultErr error) *naughtyDisk } func (d *naughtyDisk) String() string { - return d.String() + return d.disk.String() } func (d *naughtyDisk) calcError() (err error) { diff --git a/cmd/object-common.go b/cmd/object-common.go index 35d98e912..c043fa697 100644 --- a/cmd/object-common.go +++ b/cmd/object-common.go @@ -55,13 +55,47 @@ func isErrIgnored(err error, ignoredErrs []error) bool { return false } -// House keeping code needed for FS. -func fsHouseKeeping(storageDisk StorageAPI) error { - // Cleanup all temp entries upon start. - err := cleanupDir(storageDisk, minioMetaBucket, tmpMetaPrefix) - if err != nil { +// House keeping code for FS/XL and distributed Minio setup. +func houseKeeping(storageDisks []StorageAPI) error { + var wg = &sync.WaitGroup{} + + // Initialize errs to collect errors inside go-routine. + var errs = make([]error, len(storageDisks)) + + // Initialize all disks in parallel. + for index, disk := range storageDisks { + if disk == nil || !isLocalStorage(disk.String()) { + continue + } + wg.Add(1) + go func(index int, disk StorageAPI) { + // Indicate this wait group is done. + defer wg.Done() + + // Cleanup all temp entries upon start. + err := cleanupDir(disk, minioMetaBucket, tmpMetaPrefix) + if err != nil { + switch errorCause(err) { + case errDiskNotFound, errVolumeNotFound, errFileNotFound: + default: + errs[index] = err + } + } + }(index, disk) + } + + // Wait for all cleanup to finish. + wg.Wait() + + // Return upon first error. + for _, err := range errs { + if err == nil { + continue + } return toObjectErr(err, minioMetaBucket, tmpMetaPrefix) } + + // Return success here. return nil } @@ -170,53 +204,6 @@ func initMetaVolume(storageDisks []StorageAPI) error { return nil } -// House keeping code needed for XL. -func xlHouseKeeping(storageDisks []StorageAPI) error { - // This happens for the first time, but keep this here since this - // is the only place where it can be made expensive optimizing all - // other calls. Create metavolume. - var wg = &sync.WaitGroup{} - - // Initialize errs to collect errors inside go-routine. - var errs = make([]error, len(storageDisks)) - - // Initialize all disks in parallel. - for index, disk := range storageDisks { - if disk == nil { - continue - } - wg.Add(1) - go func(index int, disk StorageAPI) { - // Indicate this wait group is done. - defer wg.Done() - - // Cleanup all temp entries upon start. - err := cleanupDir(disk, minioMetaBucket, tmpMetaPrefix) - if err != nil { - switch errorCause(err) { - case errDiskNotFound, errVolumeNotFound, errFileNotFound: - default: - errs[index] = err - } - } - }(index, disk) - } - - // Wait for all cleanup to finish. - wg.Wait() - - // Return upon first error. - for _, err := range errs { - if err == nil { - continue - } - return toObjectErr(err, minioMetaBucket, tmpMetaPrefix) - } - - // Return success here. - return nil -} - // Cleanup a directory recursively. func cleanupDir(storage StorageAPI, volume, dirPath string) error { var delFunc func(string) error diff --git a/cmd/object-common_test.go b/cmd/object-common_test.go new file mode 100644 index 000000000..f39379afe --- /dev/null +++ b/cmd/object-common_test.go @@ -0,0 +1,99 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "sync" + "testing" +) + +func TestHouseKeeping(t *testing.T) { + fsDirs, err := getRandomDisks(8) + if err != nil { + t.Fatalf("Failed to create disks for storage layer %v", err) + } + defer removeRoots(fsDirs) + + noSpaceDirs, err := getRandomDisks(8) + if err != nil { + t.Fatalf("Failed to create disks for storage layer %v", err) + } + defer removeRoots(noSpaceDirs) + + properStorage := []StorageAPI{} + for _, fs := range fsDirs { + sd, err := newPosix(fs) + if err != nil { + t.Fatalf("Failed to create a local disk-based storage layer %v", err) + } + properStorage = append(properStorage, sd) + } + + noSpaceBackend := []StorageAPI{} + for _, noSpaceFS := range noSpaceDirs { + sd, err := newPosix(noSpaceFS) + if err != nil { + t.Fatalf("Failed to create a local disk-based storage layer %v", err) + } + noSpaceBackend = append(noSpaceBackend, sd) + } + noSpaceStorage := prepareNErroredDisks(noSpaceBackend, 5, errDiskFull, t) + + // Create .minio.sys/tmp directory on all disks. + wg := sync.WaitGroup{} + errs := make([]error, len(properStorage)) + for i, store := range properStorage { + wg.Add(1) + go func(index int, store StorageAPI) { + defer wg.Done() + errs[index] = store.MakeVol(minioMetaBucket) + if errs[index] != nil { + return + } + errs[index] = store.MakeVol(pathJoin(minioMetaBucket, tmpMetaPrefix)) + if errs[index] != nil { + return + } + + errs[index] = store.AppendFile(pathJoin(minioMetaBucket, tmpMetaPrefix), "hello.txt", []byte("hello")) + }(i, store) + } + wg.Wait() + for i := range errs { + if errs[i] != nil { + t.Fatalf("Failed to create .minio.sys/tmp directory on disk %v %v", + properStorage[i], errs[i]) + } + } + + nilDiskStorage := []StorageAPI{nil, nil, nil, nil, nil, nil, nil, nil} + testCases := []struct { + store []StorageAPI + expectedErr error + }{ + {properStorage, nil}, + {noSpaceStorage, StorageFull{}}, + {nilDiskStorage, nil}, + } + for i, test := range testCases { + actualErr := errorCause(houseKeeping(test.store)) + if actualErr != test.expectedErr { + t.Errorf("Test %d - actual error is %#v, expected error was %#v", + i+1, actualErr, test.expectedErr) + } + } +} diff --git a/cmd/server-main.go b/cmd/server-main.go index bd12f1f53..f37fa22bf 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -313,6 +313,9 @@ func serverMain(c *cli.Context) { // Check 'server' cli arguments. storageDisks := validateDisks(disks, ignoredDisks) + // Cleanup objects that weren't successfully written into the namespace. + fatalIf(houseKeeping(storageDisks), "Unable to purge temporary files.") + // If https. tls := isSSL() diff --git a/cmd/xl-v1.go b/cmd/xl-v1.go index 4020d36a1..4a9f2d580 100644 --- a/cmd/xl-v1.go +++ b/cmd/xl-v1.go @@ -111,11 +111,6 @@ func newXLObjects(storageDisks []StorageAPI) (ObjectLayer, error) { return nil, errInvalidArgument } - // Runs house keeping code, like t, cleaning up tmp files etc. - if err := xlHouseKeeping(storageDisks); err != nil { - return nil, err - } - readQuorum := len(storageDisks) / 2 writeQuorum := len(storageDisks)/2 + 1