pkg/fs: for locks, prefer defer and read-only ops

This commit prefers the use of 'defer' for fs.Unlock (and fs.RUnlock)
because it is more idiomatic Go and reduces repetition in the code,
lending to a cleaner code base.

It also switches a few uses of the lock to read-only locks, which should
improve performance of those functions dramatically in certain contexts.
master
Brendan Ashworth 9 years ago
parent d54a8a9c07
commit 294ea814bf
  1. 12
      pkg/fs/fs-bucket-acl.go
  2. 12
      pkg/fs/fs-bucket.go
  3. 11
      pkg/fs/fs-multipart.go

@ -18,8 +18,8 @@ package fs
// IsPrivateBucket - is private bucket // IsPrivateBucket - is private bucket
func (fs Filesystem) IsPrivateBucket(bucket string) bool { func (fs Filesystem) IsPrivateBucket(bucket string) bool {
fs.rwLock.Lock() fs.rwLock.RLock()
defer fs.rwLock.Unlock() defer fs.rwLock.RUnlock()
bucketMetadata, ok := fs.buckets.Metadata[bucket] bucketMetadata, ok := fs.buckets.Metadata[bucket]
if !ok { if !ok {
return true return true
@ -29,8 +29,8 @@ func (fs Filesystem) IsPrivateBucket(bucket string) bool {
// IsPublicBucket - is public bucket // IsPublicBucket - is public bucket
func (fs Filesystem) IsPublicBucket(bucket string) bool { func (fs Filesystem) IsPublicBucket(bucket string) bool {
fs.rwLock.Lock() fs.rwLock.RLock()
defer fs.rwLock.Unlock() defer fs.rwLock.RUnlock()
bucketMetadata, ok := fs.buckets.Metadata[bucket] bucketMetadata, ok := fs.buckets.Metadata[bucket]
if !ok { if !ok {
return true return true
@ -40,8 +40,8 @@ func (fs Filesystem) IsPublicBucket(bucket string) bool {
// IsReadOnlyBucket - is read only bucket // IsReadOnlyBucket - is read only bucket
func (fs Filesystem) IsReadOnlyBucket(bucket string) bool { func (fs Filesystem) IsReadOnlyBucket(bucket string) bool {
fs.rwLock.Lock() fs.rwLock.RLock()
defer fs.rwLock.Unlock() defer fs.rwLock.RUnlock()
bucketMetadata, ok := fs.buckets.Metadata[bucket] bucketMetadata, ok := fs.buckets.Metadata[bucket]
if !ok { if !ok {
return true return true

@ -58,12 +58,12 @@ func (fs Filesystem) DeleteBucket(bucket string) *probe.Error {
// Critical region hold write lock. // Critical region hold write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
delete(fs.buckets.Metadata, bucket) delete(fs.buckets.Metadata, bucket)
if err := saveBucketsMetadata(*fs.buckets); err != nil { if err := saveBucketsMetadata(*fs.buckets); err != nil {
fs.rwLock.Unlock()
return err.Trace(bucket) return err.Trace(bucket)
} }
fs.rwLock.Unlock()
return nil return nil
} }
@ -171,12 +171,12 @@ func (fs Filesystem) MakeBucket(bucket, acl string) *probe.Error {
// Critical region hold a write lock. // Critical region hold a write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
fs.buckets.Metadata[bucket] = bucketMetadata fs.buckets.Metadata[bucket] = bucketMetadata
if err := saveBucketsMetadata(*fs.buckets); err != nil { if err := saveBucketsMetadata(*fs.buckets); err != nil {
fs.rwLock.Unlock()
return err.Trace(bucket) return err.Trace(bucket)
} }
fs.rwLock.Unlock()
return nil return nil
} }
@ -266,11 +266,11 @@ func (fs Filesystem) SetBucketMetadata(bucket string, metadata map[string]string
// Critical region handle write lock. // Critical region handle write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
fs.buckets.Metadata[bucket] = bucketMetadata fs.buckets.Metadata[bucket] = bucketMetadata
if err := saveBucketsMetadata(*fs.buckets); err != nil { if err := saveBucketsMetadata(*fs.buckets); err != nil {
fs.rwLock.Unlock()
return err.Trace(bucket) return err.Trace(bucket)
} }
fs.rwLock.Unlock()
return nil return nil
} }

@ -276,6 +276,7 @@ func (fs Filesystem) NewMultipartUpload(bucket, object string) (string, *probe.E
// Critical region requiring write lock. // Critical region requiring write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
// Initialize multipart session. // Initialize multipart session.
mpartSession := &MultipartSession{} mpartSession := &MultipartSession{}
mpartSession.TotalParts = 0 mpartSession.TotalParts = 0
@ -288,10 +289,8 @@ func (fs Filesystem) NewMultipartUpload(bucket, object string) (string, *probe.E
fs.multiparts.ActiveSession[uploadID] = mpartSession fs.multiparts.ActiveSession[uploadID] = mpartSession
if err := saveMultipartsSession(*fs.multiparts); err != nil { if err := saveMultipartsSession(*fs.multiparts); err != nil {
fs.rwLock.Unlock()
return "", err.Trace(objectPath) return "", err.Trace(objectPath)
} }
fs.rwLock.Unlock()
return uploadID, nil return uploadID, nil
} }
@ -445,12 +444,12 @@ func (fs Filesystem) CreateObjectPart(bucket, object, uploadID, expectedMD5Sum s
// Critical region requiring write lock. // Critical region requiring write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
fs.multiparts.ActiveSession[uploadID] = deserializedMultipartSession fs.multiparts.ActiveSession[uploadID] = deserializedMultipartSession
if err := saveMultipartsSession(*fs.multiparts); err != nil { if err := saveMultipartsSession(*fs.multiparts); err != nil {
fs.rwLock.Unlock()
return "", err.Trace(partPathPrefix) return "", err.Trace(partPathPrefix)
} }
fs.rwLock.Unlock()
// Return etag. // Return etag.
return partMetadata.ETag, nil return partMetadata.ETag, nil
@ -693,11 +692,11 @@ func (fs Filesystem) AbortMultipartUpload(bucket, object, uploadID string) *prob
// Critical region requiring write lock. // Critical region requiring write lock.
fs.rwLock.Lock() fs.rwLock.Lock()
defer fs.rwLock.Unlock()
delete(fs.multiparts.ActiveSession, uploadID) delete(fs.multiparts.ActiveSession, uploadID)
if err := saveMultipartsSession(*fs.multiparts); err != nil { if err := saveMultipartsSession(*fs.multiparts); err != nil {
fs.rwLock.Unlock()
return err.Trace(partPathPrefix) return err.Trace(partPathPrefix)
} }
fs.rwLock.Unlock()
return nil return nil
} }

Loading…
Cancel
Save