fix: fork os.MkdirAll to optimize cases where parent exists (#15379)

a/b/c/d/ where `a/b/c/` exists results in additional syscalls
such as an Lstat() call to verify if the `a/b/c/` exists
and its a directory.

We do not need to do this on MinIO since the parent prefixes
if exist, we can simply return success without spending
additional syscalls.

Also this implementation attempts to simply use Access() calls
to avoid os.Stat() calls since the latter does memory allocation
for things we do not need to use.

Access() is simpler since we have a predictable structure on
the backend and we know exactly how our path structures are.
This commit is contained in:
Harshavardhana 2022-07-24 00:43:11 -07:00 committed by GitHub
parent b2f4948bbe
commit 7725425e05
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 93 additions and 18 deletions

View file

@ -187,6 +187,8 @@ func (fsi *fsIOPool) Create(path string) (wlk *lock.LockedFile, err error) {
return nil, errFileAccessDenied
case isSysErrIsDir(err):
return nil, errIsNotRegular
case isSysErrNotDir(err):
return nil, errFileAccessDenied
case isSysErrPathNotFound(err):
return nil, errFileAccessDenied
default:

View file

@ -35,6 +35,7 @@ type osMetric uint8
const (
osMetricRemoveAll osMetric = iota
osMetricMkdirAll
osMetricMkdir
osMetricRename
osMetricOpenFile
osMetricOpen
@ -114,10 +115,16 @@ func RemoveAll(dirPath string) error {
return os.RemoveAll(dirPath)
}
// Mkdir captures time taken to call os.Mkdir
func Mkdir(dirPath string, mode os.FileMode) error {
defer updateOSMetrics(osMetricMkdir, dirPath)()
return os.Mkdir(dirPath, mode)
}
// MkdirAll captures time taken to call os.MkdirAll
func MkdirAll(dirPath string, mode os.FileMode) error {
defer updateOSMetrics(osMetricMkdirAll, dirPath)()
return os.MkdirAll(dirPath, mode)
return osMkdirAll(dirPath, mode)
}
// Rename captures time taken to call os.Rename

View file

@ -104,7 +104,7 @@ func reliableMkdirAll(dirPath string, mode os.FileMode) (err error) {
i := 0
for {
// Creates all the parent directories, with mode 0777 mkdir honors system umask.
if err = MkdirAll(dirPath, mode); err != nil {
if err = osMkdirAll(dirPath, mode); err != nil {
// Retry only for the first retryable error.
if osIsNotExist(err) && i == 0 {
i++
@ -166,6 +166,7 @@ func reliableRename(srcFilePath, dstFilePath string) (err error) {
if err = reliableMkdirAll(path.Dir(dstFilePath), 0o777); err != nil {
return err
}
i := 0
for {
// After a successful parent directory create attempt a renameAll.

View file

@ -31,6 +31,10 @@ func access(name string) error {
return err
}
func osMkdirAll(dirPath string, perm os.FileMode) error {
return os.MkdirAll(dirPath, perm)
}
// readDirFn applies the fn() function on each entries at dirPath, doesn't recurse into
// the directory itself, if the dirPath doesn't exist this function doesn't return
// an error.

View file

@ -38,6 +38,55 @@ func access(name string) error {
return nil
}
// Forked from Golang but chooses fast path upon os.Mkdir()
// error to avoid os.Lstat() call.
//
// osMkdirAll creates a directory named path,
// along with any necessary parents, and returns nil,
// or else returns an error.
// The permission bits perm (before umask) are used for all
// directories that MkdirAll creates.
// If path is already a directory, MkdirAll does nothing
// and returns nil.
func osMkdirAll(dirPath string, perm os.FileMode) error {
// Fast path: if we can tell whether path is a directory or file, stop with success or error.
err := Access(dirPath)
if err == nil {
return nil
}
if !osIsNotExist(err) {
return &os.PathError{Op: "mkdir", Path: dirPath, Err: err}
}
// Slow path: make sure parent exists and then call Mkdir for path.
i := len(dirPath)
for i > 0 && os.IsPathSeparator(dirPath[i-1]) { // Skip trailing path separator.
i--
}
j := i
for j > 0 && !os.IsPathSeparator(dirPath[j-1]) { // Scan backward over element.
j--
}
if j > 1 {
// Create parent.
if err = osMkdirAll(dirPath[:j-1], perm); err != nil {
return err
}
}
// Parent now exists; invoke Mkdir and use its result.
if err = Mkdir(dirPath, perm); err != nil {
if osIsExist(err) {
return nil
}
return err
}
return nil
}
// The buffer must be at least a block long.
// refer https://github.com/golang/go/issues/24015
const blockSize = 8 << 10 // 8192

View file

@ -31,6 +31,10 @@ func access(name string) error {
return err
}
func osMkdirAll(dirPath string, perm os.FileMode) error {
return os.MkdirAll(dirPath, perm)
}
// readDirFn applies the fn() function on each entries at dirPath, doesn't recurse into
// the directory itself, if the dirPath doesn't exist this function doesn't return
// an error.

View file

@ -10,24 +10,25 @@ func _() {
var x [1]struct{}
_ = x[osMetricRemoveAll-0]
_ = x[osMetricMkdirAll-1]
_ = x[osMetricRename-2]
_ = x[osMetricOpenFile-3]
_ = x[osMetricOpen-4]
_ = x[osMetricOpenFileDirectIO-5]
_ = x[osMetricLstat-6]
_ = x[osMetricRemove-7]
_ = x[osMetricStat-8]
_ = x[osMetricAccess-9]
_ = x[osMetricCreate-10]
_ = x[osMetricReadDirent-11]
_ = x[osMetricFdatasync-12]
_ = x[osMetricSync-13]
_ = x[osMetricLast-14]
_ = x[osMetricMkdir-2]
_ = x[osMetricRename-3]
_ = x[osMetricOpenFile-4]
_ = x[osMetricOpen-5]
_ = x[osMetricOpenFileDirectIO-6]
_ = x[osMetricLstat-7]
_ = x[osMetricRemove-8]
_ = x[osMetricStat-9]
_ = x[osMetricAccess-10]
_ = x[osMetricCreate-11]
_ = x[osMetricReadDirent-12]
_ = x[osMetricFdatasync-13]
_ = x[osMetricSync-14]
_ = x[osMetricLast-15]
}
const _osMetric_name = "RemoveAllMkdirAllRenameOpenFileOpenOpenFileDirectIOLstatRemoveStatAccessCreateReadDirentFdatasyncSyncLast"
const _osMetric_name = "RemoveAllMkdirAllMkdirRenameOpenFileOpenOpenFileDirectIOLstatRemoveStatAccessCreateReadDirentFdatasyncSyncLast"
var _osMetric_index = [...]uint8{0, 9, 17, 23, 31, 35, 51, 56, 62, 66, 72, 78, 88, 97, 101, 105}
var _osMetric_index = [...]uint8{0, 9, 17, 22, 28, 36, 40, 56, 61, 67, 71, 77, 83, 93, 102, 106, 110}
func (i osMetric) String() string {
if i >= osMetric(len(_osMetric_index)-1) {

View file

@ -1647,6 +1647,8 @@ func (s *xlStorage) openFileSync(filePath string, mode int) (f *os.File, err err
return nil, errIsNotRegular
case osIsPermission(err):
return nil, errFileAccessDenied
case isSysErrNotDir(err):
return nil, errFileAccessDenied
case isSysErrIO(err):
return nil, errFaultyDisk
case isSysErrTooManyFiles(err):
@ -2487,8 +2489,10 @@ func (s *xlStorage) RenameFile(ctx context.Context, srcVolume, srcPath, dstVolum
return errFileAccessDenied
}
if err = Remove(dstFilePath); err != nil {
if isSysErrNotEmpty(err) {
if isSysErrNotEmpty(err) || isSysErrNotDir(err) {
return errFileAccessDenied
} else if isSysErrIO(err) {
return errFaultyDisk
}
return err
}
@ -2496,6 +2500,9 @@ func (s *xlStorage) RenameFile(ctx context.Context, srcVolume, srcPath, dstVolum
}
if err = renameAll(srcFilePath, dstFilePath); err != nil {
if isSysErrNotEmpty(err) || isSysErrNotDir(err) {
return errFileAccessDenied
}
return osErrToFileErr(err)
}