fs: Hold `format.json` readLock ref to avoid GC. (#4532)

Looks like if we follow pattern such as

```
_ = rlk
```

Go can potentially kick in GC and close the fd when
the reference is lost, only speculation is that
the cause here is `SetFinalizer` which is set on
`os.close()` internally in `os` stdlib.

This is unexpected and unsual endeavour for Go, but
we have to make sure the reference is never lost
and always dies with the server.

Fixes #4530
master
Harshavardhana 7 years ago committed by GitHub
parent c8947af227
commit 353f2d3a6e
  1. 48
      cmd/fs-v1-metadata.go
  2. 18
      cmd/fs-v1.go
  3. 2
      pkg/lock/lock_solaris.go

@ -290,17 +290,23 @@ func checkLockedValidFormatFS(fsPath string) (*lock.RLockedFile, error) {
return rlk, traceError(err) return rlk, traceError(err)
} }
// Writes the new format.json if unformatted, // Creates a new format.json if unformatted.
// otherwise closes the input locked file func createFormatFS(fsPath string) error {
// and returns any error. fsFormatPath := pathJoin(fsPath, minioMetaBucket, formatConfigFile)
func writeFormatFS(lk *lock.LockedFile) error {
// Attempt a write lock on formatConfigFile `format.json`
// file stored in minioMetaBucket(.minio.sys) directory.
lk, err := lock.TryLockedOpenFile(preparePath(fsFormatPath), os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
return traceError(err)
}
// Close the locked file upon return. // Close the locked file upon return.
defer lk.Close() defer lk.Close()
// Load format on disk, checks if we are unformatted // Load format on disk, checks if we are unformatted
// writes the new format.json // writes the new format.json
var format = &formatConfigV1{} var format = &formatConfigV1{}
err := format.LoadFormat(lk) err = format.LoadFormat(lk)
if errorCause(err) == errUnformattedDisk { if errorCause(err) == errUnformattedDisk {
_, err = newFSFormat().WriteTo(lk) _, err = newFSFormat().WriteTo(lk)
return err return err
@ -308,20 +314,12 @@ func writeFormatFS(lk *lock.LockedFile) error {
return err return err
} }
func initFormatFS(fsPath, fsUUID string) (err error) { func initFormatFS(fsPath string) (rlk *lock.RLockedFile, err error) {
fsFormatPath := pathJoin(fsPath, minioMetaBucket, formatConfigFile)
// Once the filesystem has initialized hold the read lock for
// the life time of the server. This is done to ensure that under
// shared backend mode for FS, remote servers do not migrate
// or cause changes on backend format.
// This loop validates format.json by holding a read lock and // This loop validates format.json by holding a read lock and
// proceeds if disk unformatted to hold non-blocking WriteLock // proceeds if disk unformatted to hold non-blocking WriteLock
// If for some reason non-blocking WriteLock fails and the error // If for some reason non-blocking WriteLock fails and the error
// is lock.ErrAlreadyLocked i.e some other process is holding a // is lock.ErrAlreadyLocked i.e some other process is holding a
// lock we retry in the loop again. // lock we retry in the loop again.
var rlk *lock.RLockedFile
for { for {
// Validate the `format.json` for expected values. // Validate the `format.json` for expected values.
rlk, err = checkLockedValidFormatFS(fsPath) rlk, err = checkLockedValidFormatFS(fsPath)
@ -330,34 +328,24 @@ func initFormatFS(fsPath, fsUUID string) (err error) {
// Holding a read lock ensures that any write lock operation // Holding a read lock ensures that any write lock operation
// is blocked if attempted in-turn avoiding corruption on // is blocked if attempted in-turn avoiding corruption on
// the backend disk. // the backend disk.
_ = rlk // Hold the lock on `format.json` until server dies. return rlk, nil
return nil
case errorCause(err) == errUnformattedDisk: case errorCause(err) == errUnformattedDisk:
// Attempt a write lock on formatConfigFile `format.json` if err = createFormatFS(fsPath); err != nil {
// file stored in minioMetaBucket(.minio.sys) directory.
var lk *lock.LockedFile
lk, err = lock.TryLockedOpenFile(preparePath(fsFormatPath), os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
// Existing write locks detected. // Existing write locks detected.
if err == lock.ErrAlreadyLocked { if errorCause(err) == lock.ErrAlreadyLocked {
// Lock already present, sleep and attempt again. // Lock already present, sleep and attempt again.
time.Sleep(100 * time.Millisecond) time.Sleep(100 * time.Millisecond)
continue continue
} }
// Unexpected error, return. // Unexpected error, return.
return traceError(err) return nil, err
} }
// Write new format. // Loop will continue to attempt a read-lock on `format.json`.
if err = writeFormatFS(lk); err != nil {
return err
}
// Loop will continue to attempt a
// read-lock on `format.json` .
default: default:
// Unhandled error return. // Unhandled error return.
return err return nil, err
} }
} }
} }

@ -42,6 +42,9 @@ type fsObjects struct {
// temporary transactions. // temporary transactions.
fsUUID string fsUUID string
// This value shouldn't be touched, once initialized.
fsFormatRlk *lock.RLockedFile // Is a read lock on `format.json`.
// FS rw pool. // FS rw pool.
rwPool *fsIOPool rwPool *fsIOPool
@ -109,6 +112,12 @@ func newFSObjectLayer(fsPath string) (ObjectLayer, error) {
return nil, fmt.Errorf("Unable to initialize '.minio.sys' meta volume, %s", err) return nil, fmt.Errorf("Unable to initialize '.minio.sys' meta volume, %s", err)
} }
// Initialize `format.json`, this function also returns.
rlk, err := initFormatFS(fsPath)
if err != nil {
return nil, err
}
// Initialize fs objects. // Initialize fs objects.
fs := &fsObjects{ fs := &fsObjects{
fsPath: fsPath, fsPath: fsPath,
@ -122,10 +131,11 @@ func newFSObjectLayer(fsPath string) (ObjectLayer, error) {
}, },
} }
// Initialize `format.json`. // Once the filesystem has initialized hold the read lock for
if err = initFormatFS(fsPath, fsUUID); err != nil { // the life time of the server. This is done to ensure that under
return nil, err // shared backend mode for FS, remote servers do not migrate
} // or cause changes on backend format.
fs.fsFormatRlk = rlk
// Initialize and load bucket policies. // Initialize and load bucket policies.
err = initBucketPolicies(fs) err = initBucketPolicies(fs)

@ -58,7 +58,7 @@ func lockedOpenFile(path string, flag int, perm os.FileMode, rlockType int) (*Lo
if err = syscall.FcntlFlock(f.Fd(), rlockType, &lock); err != nil { if err = syscall.FcntlFlock(f.Fd(), rlockType, &lock); err != nil {
f.Close() f.Close()
if err == syscall.EAGAIN { if err == syscall.EAGAIN {
err = ErrLocked err = ErrAlreadyLocked
} }
return nil, err return nil, err
} }

Loading…
Cancel
Save