MyFavMirrors/minio
1
0
Fork 0
mirror of https://github.com/minio/minio.git synced 2025-11-09 21:49:46 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix a possible race during PutObject() (#5376)

Browse Source 
Under any concurrent removeObjects in progress
might have removed the parents of the same prefix
for which there is an ongoing putObject request.
An inconsistent situation may arise as explained
below even under sufficient locking.

PutObject is almost successful at the last stage when
a temporary file is renamed to its actual namespace
at `a/b/c/object1`. Concurrently a RemoveObject is
also in progress at the same prefix for an `a/b/c/object2`.

To create the object1 at location `a/b/c` PutObject has
to create all the parents recursively.

```
a/b/c - os.MkdirAll loops through has now created
        'a/' and 'b/' about to create 'c/'
a/b/c/object2 - at this point 'c/' and 'object2'
        are deleted about to delete b/
```

Now for os.MkdirAll loop the expected situation is
that top level parent 'a/b/' exists which it created
, such that it can create 'c/' - since removeObject
and putObject do not compete for lock due to holding
locks at different resources. removeObject proceeds
to delete parent 'b/' since 'c/' is not yet present,
once deleted 'os.MkdirAll' would receive an error as
syscall.ENOENT which would fail the putObject request.

This PR tries to address this issue by implementing
a safer/guarded approach where we would retry an operation
such as `os.MkdirAll` and `os.Rename` if both operations
observe syscall.ENOENT.

Fixes #5254
This commit is contained in:
Harshavardhana
2018-01-13 09:13:02 -08:00
committed by Nitish Tiwari
parent 0bb6247056
commit 12f67d47f1
6 changed files with 211 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
cmd/fs-v1-helpers.go
View File

@@ -1,5 +1,5 @@
/*
* Minio Cloud Storage, (C) 2016 Minio, Inc.
* Minio Cloud Storage, (C) 2016, 2017, 2018 Minio, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,7 +17,6 @@
package cmd
import (
"fmt"
"io"
"os"
pathutil "path"
@@ -91,24 +90,6 @@ func fsRemoveDir(dirPath string) (err error) {
return nil
}
// Creates a new directory, parent dir is also recursively created
// if it doesn't exist.
func fsMkdirAll(dirPath string) (err error) {
if dirPath == "" {
return errors.Trace(errInvalidArgument)
}
if err = checkPathLength(dirPath); err != nil {
return errors.Trace(err)
}
if err = os.MkdirAll(dirPath, 0777); err != nil {
return errors.Trace(err)
}
return nil
}
// Creates a new directory, parent dir should exist
// otherwise returns an error. If directory already
// exists returns an error. Windows long paths
@@ -277,7 +258,7 @@ func fsCreateFile(filePath string, reader io.Reader, buf []byte, fallocSize int6
return 0, errors.Trace(err)
}
if err := os.MkdirAll(pathutil.Dir(filePath), 0777); err != nil {
if err := mkdirAll(pathutil.Dir(filePath), 0777); err != nil {
return 0, errors.Trace(err)
}
@@ -374,19 +355,16 @@ func fsRenameFile(sourcePath, destPath string) error {
if err := checkPathLength(destPath); err != nil {
return errors.Trace(err)
}
// Verify if source path exists.
if _, err := os.Stat((sourcePath)); err != nil {
if _, err := os.Stat(sourcePath); err != nil {
return osErrToFSFileErr(err)
}
if err := os.MkdirAll(pathutil.Dir(destPath), 0777); err != nil {
return errors.Trace(err)
}
if err := os.Rename((sourcePath), (destPath)); err != nil {
if isSysErrCrossDevice(err) {
return errors.Trace(fmt.Errorf("%s (%s)->(%s)", errCrossDeviceLink, sourcePath, destPath))
}
if err := renameAll(sourcePath, destPath); err != nil {
return errors.Trace(err)
}
return nil
}