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

@@ -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 {

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) {

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 {
+// 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(storageDisk, minioMetaBucket, tmpMetaPrefix)
+			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

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 <ERROR> %v", err)
+	}
+	defer removeRoots(fsDirs)
+
+	noSpaceDirs, err := getRandomDisks(8)
+	if err != nil {
+		t.Fatalf("Failed to create disks for storage layer <ERROR> %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 <ERROR> %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 <ERROR> %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 <ERROR> %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)
+		}
+	}
+}

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()
 

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