posix: Fix windows performance issues. (#3132)

Do not attempt to fetch volume/drive information for
each i/o situation. In our case we do this in all calls
`posix.go` this in-turn created a terrible situation for
windows. This issue does not affect the i/o path on Unix
platforms since statvfs calls are in the range of micro
seconds on these platforms.

This verification is only needed during startup and we
let things fail at a later stage on windows.

cmd/posix-errors.go

@@ -42,7 +42,7 @@ func isSysErrIO(err error) bool {
 	return err != nil && err == syscall.EIO
 }
 
-// Check if the given error corresponds to ENOTDIR (is not a directory)
+// Check if the given error corresponds to ENOTDIR (is not a directory).
 func isSysErrNotDir(err error) bool {
 	if pathErr, ok := err.(*os.PathError); ok {
 		switch pathErr.Err {
@@ -53,8 +53,19 @@ func isSysErrNotDir(err error) bool {
 	return false
 }
 
+// Check if the given error corresponds to the ENAMETOOLONG (name too long).
+func isSysErrTooLong(err error) bool {
+	if pathErr, ok := err.(*os.PathError); ok {
+		switch pathErr.Err {
+		case syscall.ENAMETOOLONG:
+			return true
+		}
+	}
+	return false
+}
+
 // Check if the given error corresponds to ENOTEMPTY for unix
-// and ERROR_DIR_NOT_EMPTY for windows (directory not empty)
+// and ERROR_DIR_NOT_EMPTY for windows (directory not empty).
 func isSysErrNotEmpty(err error) bool {
 	if pathErr, ok := err.(*os.PathError); ok {
 		if runtime.GOOS == "windows" {

cmd/posix-errors_test.go

@@ -0,0 +1,57 @@
+/*
+ * 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 (
+	"os"
+	"runtime"
+	"syscall"
+	"testing"
+)
+
+func TestSysErrors(t *testing.T) {
+	pathErr := &os.PathError{Err: syscall.ENAMETOOLONG}
+	ok := isSysErrTooLong(pathErr)
+	if !ok {
+		t.Fatalf("Unexpected error expecting %s", syscall.ENAMETOOLONG)
+	}
+	pathErr = &os.PathError{Err: syscall.ENOTDIR}
+	ok = isSysErrNotDir(pathErr)
+	if !ok {
+		t.Fatalf("Unexpected error expecting %s", syscall.ENOTDIR)
+	}
+	if runtime.GOOS != "windows" {
+		pathErr = &os.PathError{Err: syscall.ENOTEMPTY}
+		ok = isSysErrNotEmpty(pathErr)
+		if !ok {
+			t.Fatalf("Unexpected error expecting %s", syscall.ENOTEMPTY)
+		}
+	} else {
+		pathErr = &os.PathError{Err: syscall.Errno(0x91)}
+		ok = isSysErrNotEmpty(pathErr)
+		if !ok {
+			t.Fatal("Unexpected error expecting 0x91")
+		}
+	}
+	if runtime.GOOS == "windows" {
+		pathErr = &os.PathError{Err: syscall.Errno(0x03)}
+		ok = isSysErrPathNotFound(pathErr)
+		if !ok {
+			t.Fatal("Unexpected error expecting 0x03")
+		}
+	}
+}

cmd/posix.go

@@ -34,8 +34,8 @@ import (
 )
 
 const (
-	fsMinFreeSpace         = 1024 * 1024 * 1024
-	fsMinFreeInodesPercent = 5
+	fsMinFreeSpace    = 1024 * 1024 * 1024 // Min 1GiB free space.
+	fsMinFreeInodes   = 10000              // Min 10000.
 	maxAllowedIOError = 5
 )
 
@@ -57,6 +57,9 @@ func checkPathLength(pathName string) error {
 		return errFileNameTooLong
 	}
 
+	// Convert any '\' to '/'.
+	pathName = filepath.ToSlash(pathName)
+
 	// Check each path segment length is > 255
 	for len(pathName) > 0 && pathName != "." && pathName != "/" {
 		dir, file := slashpath.Dir(pathName), slashpath.Base(pathName)
@@ -111,7 +114,7 @@ func newPosix(path string) (StorageAPI, error) {
 	fs := &posix{
 		diskPath:      diskPath,
 		minFreeSpace:  fsMinFreeSpace,
-		minFreeInodes: fsMinFreeInodesPercent,
+		minFreeInodes: fsMinFreeInodes,
 		// 1MiB buffer pool for posix internal operations.
 		pool: sync.Pool{
 			New: func() interface{} {
@@ -120,17 +123,21 @@ func newPosix(path string) (StorageAPI, error) {
 			},
 		},
 	}
-	st, err := os.Stat(preparePath(diskPath))
-	if err != nil {
+	fi, err := os.Stat(preparePath(diskPath))
+	if err == nil {
+		if !fi.IsDir() {
+			return nil, syscall.ENOTDIR
+		}
+	}
 	if os.IsNotExist(err) {
 		// Disk not found create it.
-			err = os.MkdirAll(diskPath, 0777)
-			return fs, err
+		err = os.MkdirAll(preparePath(diskPath), 0777)
+		if err != nil {
+			return nil, err
 		}
-		return fs, err
 	}
-	if !st.IsDir() {
-		return fs, syscall.ENOTDIR
+	if err = fs.checkDiskFree(); err != nil {
+		return nil, err
 	}
 	return fs, nil
 }
@@ -150,7 +157,14 @@ func getDiskInfo(diskPath string) (di disk.Info, err error) {
 
 // checkDiskFree verifies if disk path has sufficient minimum free disk space and files.
 func (s *posix) checkDiskFree() (err error) {
-	di, err := getDiskInfo(s.diskPath)
+	// We don't validate disk space or inode utilization on windows.
+	// Each windows calls to 'GetVolumeInformationW' takes around 3-5seconds.
+	if runtime.GOOS == "windows" {
+		return nil
+	}
+
+	var di disk.Info
+	di, err = getDiskInfo(preparePath(s.diskPath))
 	if err != nil {
 		return err
 	}
@@ -166,8 +180,8 @@ func (s *posix) checkDiskFree() (err error) {
 	// Allow for the available disk to be separately validate and we will validate inodes only if
 	// total inodes are provided by the underlying filesystem.
 	if di.Files != 0 {
-		availableFiles := 100 * float64(di.Ffree) / float64(di.Files)
-		if int64(availableFiles) <= s.minFreeInodes {
+		availableFiles := int64(di.Ffree)
+		if availableFiles <= s.minFreeInodes {
 			return errDiskFull
 		}
 	}
@@ -184,7 +198,7 @@ func (s *posix) String() string {
 // DiskInfo provides current information about disk space usage,
 // total free inodes and underlying filesystem.
 func (s *posix) DiskInfo() (info disk.Info, err error) {
-	return getDiskInfo(s.diskPath)
+	return getDiskInfo(preparePath(s.diskPath))
 }
 
 // getVolDir - will convert incoming volume names to
@@ -199,6 +213,20 @@ func (s *posix) getVolDir(volume string) (string, error) {
 	return volumeDir, nil
 }
 
+// checkDiskFound - validates if disk is available,
+// returns errDiskNotFound if not found.
+func (s *posix) checkDiskFound() (err error) {
+	_, err = os.Stat(preparePath(s.diskPath))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return errDiskNotFound
+		} else if isSysErrTooLong(err) {
+			return errFileNameTooLong
+		}
+	}
+	return err
+}
+
 // Make a volume entry.
 func (s *posix) MakeVol(volume string) (err error) {
 	defer func() {
@@ -211,8 +239,7 @@ func (s *posix) MakeVol(volume string) (err error) {
 		return errFaultyDisk
 	}
 
-	// Validate if disk is free.
-	if err = s.checkDiskFree(); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return err
 	}
 
@@ -246,7 +273,11 @@ func (s *posix) ListVols() (volsInfo []VolInfo, err error) {
 		return nil, errFaultyDisk
 	}
 
-	volsInfo, err = listVols(s.diskPath)
+	if err = s.checkDiskFound(); err != nil {
+		return nil, err
+	}
+
+	volsInfo, err = listVols(preparePath(s.diskPath))
 	if err != nil {
 		return nil, err
 	}
@@ -306,8 +337,7 @@ func (s *posix) StatVol(volume string) (volInfo VolInfo, err error) {
 		return VolInfo{}, errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return VolInfo{}, err
 	}
 
@@ -346,8 +376,7 @@ func (s *posix) DeleteVol(volume string) (err error) {
 		return errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return err
 	}
 
@@ -381,8 +410,7 @@ func (s *posix) ListDir(volume, dirPath string) (entries []string, err error) {
 		return nil, errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return nil, err
 	}
 
@@ -419,8 +447,7 @@ func (s *posix) ReadAll(volume, path string) (buf []byte, err error) {
 		return nil, errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return nil, err
 	}
 
@@ -439,7 +466,7 @@ func (s *posix) ReadAll(volume, path string) (buf []byte, err error) {
 
 	// Validate file path length, before reading.
 	filePath := pathJoin(volumeDir, path)
-	if err = checkPathLength(filePath); err != nil {
+	if err = checkPathLength(preparePath(filePath)); err != nil {
 		return nil, err
 	}
 
@@ -483,8 +510,7 @@ func (s *posix) ReadFile(volume string, path string, offset int64, buf []byte) (
 		return 0, errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return 0, err
 	}
 
@@ -503,7 +529,7 @@ func (s *posix) ReadFile(volume string, path string, offset int64, buf []byte) (
 
 	// Validate effective path length before reading.
 	filePath := pathJoin(volumeDir, path)
-	if err = checkPathLength(filePath); err != nil {
+	if err = checkPathLength(preparePath(filePath)); err != nil {
 		return 0, err
 	}
 
@@ -557,8 +583,7 @@ func (s *posix) createFile(volume, path string) (f *os.File, err error) {
 		return nil, errFaultyDisk
 	}
 
-	// Validate if disk is free.
-	if err = s.checkDiskFree(); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return nil, err
 	}
 
@@ -576,7 +601,7 @@ func (s *posix) createFile(volume, path string) (f *os.File, err error) {
 	}
 
 	filePath := pathJoin(volumeDir, path)
-	if err = checkPathLength(filePath); err != nil {
+	if err = checkPathLength(preparePath(filePath)); err != nil {
 		return nil, err
 	}
 
@@ -632,6 +657,11 @@ func (s *posix) PrepareFile(volume, path string, fileSize int64) (err error) {
 		return errFaultyDisk
 	}
 
+	// Validate if disk is indeed free.
+	if err = s.checkDiskFree(); err != nil {
+		return err
+	}
+
 	// Create file if not found
 	w, err := s.createFile(volume, path)
 	if err != nil {
@@ -705,8 +735,7 @@ func (s *posix) StatFile(volume, path string) (file FileInfo, err error) {
 		return FileInfo{}, errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return FileInfo{}, err
 	}
 
@@ -724,7 +753,7 @@ func (s *posix) StatFile(volume, path string) (file FileInfo, err error) {
 	}
 
 	filePath := slashpath.Join(volumeDir, path)
-	if err = checkPathLength(filePath); err != nil {
+	if err = checkPathLength(preparePath(filePath)); err != nil {
 		return FileInfo{}, err
 	}
 	st, err := os.Stat(preparePath(filePath))
@@ -802,8 +831,7 @@ func (s *posix) DeleteFile(volume, path string) (err error) {
 		return errFaultyDisk
 	}
 
-	// Check disk availability.
-	if _, err = getDiskInfo(s.diskPath); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return err
 	}
 
@@ -823,7 +851,7 @@ func (s *posix) DeleteFile(volume, path string) (err error) {
 	// Following code is needed so that we retain "/" suffix if any in
 	// path argument.
 	filePath := pathJoin(volumeDir, path)
-	if err = checkPathLength(filePath); err != nil {
+	if err = checkPathLength(preparePath(filePath)); err != nil {
 		return err
 	}
 
@@ -843,8 +871,7 @@ func (s *posix) RenameFile(srcVolume, srcPath, dstVolume, dstPath string) (err e
 		return errFaultyDisk
 	}
 
-	// Validate if disk is free.
-	if err = s.checkDiskFree(); err != nil {
+	if err = s.checkDiskFound(); err != nil {
 		return err
 	}
 
@@ -878,11 +905,11 @@ func (s *posix) RenameFile(srcVolume, srcPath, dstVolume, dstPath string) (err e
 		return errFileAccessDenied
 	}
 	srcFilePath := slashpath.Join(srcVolumeDir, srcPath)
-	if err = checkPathLength(srcFilePath); err != nil {
+	if err = checkPathLength(preparePath(srcFilePath)); err != nil {
 		return err
 	}
 	dstFilePath := slashpath.Join(dstVolumeDir, dstPath)
-	if err = checkPathLength(dstFilePath); err != nil {
+	if err = checkPathLength(preparePath(dstFilePath)); err != nil {
 		return err
 	}
 	if srcIsDir {

cmd/posix_test.go

@@ -536,23 +536,6 @@ func TestListVols(t *testing.T) {
 	if _, err = posixStorage.ListVols(); err != errDiskNotFound {
 		t.Errorf("Expected to fail with \"%s\", but instead failed with \"%s\"", errDiskNotFound, err)
 	}
-	// creating a new posix instance.
-	posixStorage, path, err = newPosixTestSetup()
-	if err != nil {
-		t.Fatalf("Unable to create posix test setup, %s", err)
-	}
-	defer removeAll(path)
-	// adding the segment of the path length of the volume to be > 255.
-	if posixType, ok := posixStorage.(*posix); ok {
-		// setting the disk Path with name whose segment length > 255.
-		posixType.diskPath = "my-vol-name-0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001"
-	} else {
-		t.Errorf("Expected the StorageAPI to be of type *posix")
-	}
-	if _, err = posixStorage.ListVols(); err != errFileNameTooLong {
-		t.Errorf("Expected to fail with \"%s\", but instead failed with \"%s\"", errFileNameTooLong, err)
-	}
-
 }
 
 // TestPosixListDir -  Tests validate the directory listing functionality provided by posix.ListDir .

pkg/disk/stat_windows.go

@@ -24,6 +24,19 @@ import (
 	"unsafe"
 )
 
+var (
+	kernel32 = syscall.NewLazyDLL("kernel32.dll")
+
+	// GetDiskFreeSpaceEx - https://msdn.microsoft.com/en-us/library/windows/desktop/aa364937(v=vs.85).aspx
+	// Retrieves information about the amount of space that is available on a disk volume,
+	// which is the total amount of space, the total amount of free space, and the total
+	// amount of free space available to the user that is associated with the calling thread.
+	GetDiskFreeSpaceEx = kernel32.NewProc("GetDiskFreeSpaceExW")
+	// GetDiskFreeSpace - https://msdn.microsoft.com/en-us/library/windows/desktop/aa364935(v=vs.85).aspx
+	// Retrieves information about the specified disk, including the amount of free space on the disk.
+	GetDiskFreeSpace = kernel32.NewProc("GetDiskFreeSpaceW")
+)
+
 // GetInfo returns total and free bytes available in a directory, e.g. `C:\`.
 // It returns free space available to the user (including quota limitations)
 //
@@ -34,13 +47,6 @@ func GetInfo(path string) (info Info, err error) {
 		return Info{}, err
 	}
 
-	dll := syscall.MustLoadDLL("kernel32.dll")
-	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364937(v=vs.85).aspx
-	// Retrieves information about the amount of space that is available on a disk volume,
-	// which is the total amount of space, the total amount of free space, and the total
-	// amount of free space available to the user that is associated with the calling thread.
-	GetDiskFreeSpaceEx := dll.MustFindProc("GetDiskFreeSpaceExW")
-
 	lpFreeBytesAvailable := int64(0)
 	lpTotalNumberOfBytes := int64(0)
 	lpTotalNumberOfFreeBytes := int64(0)
@@ -61,10 +67,6 @@ func GetInfo(path string) (info Info, err error) {
 	info.Free = int64(lpFreeBytesAvailable)
 	info.FSType = getFSType(path)
 
-	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364935(v=vs.85).aspx
-	// Retrieves information about the specified disk, including the amount of free space on the disk.
-	GetDiskFreeSpace := dll.MustFindProc("GetDiskFreeSpaceW")
-
 	// Return values of GetDiskFreeSpace()
 	lpSectorsPerCluster := uint32(0)
 	lpBytesPerSector := uint32(0)

pkg/disk/type_windows.go

@@ -24,11 +24,13 @@ import (
 	"unsafe"
 )
 
+var (
+	// GetVolumeInformation provides windows drive volume information.
+	GetVolumeInformation = kernel32.NewProc("GetVolumeInformationW")
+)
+
 // getFSType returns the filesystem type of the underlying mounted filesystem
 func getFSType(path string) string {
-	dll := syscall.MustLoadDLL("kernel32.dll")
-	GetVolumeInformation := dll.MustFindProc("GetVolumeInformationW")
-
 	var volumeNameSize uint32 = 260
 	var nFileSystemNameSize, lpVolumeSerialNumber uint32
 	var lpFileSystemFlags, lpMaximumComponentLength uint32