From 37cbcae6ba1567217bc25b27768ebac513e1dfbd Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 1 Sep 2016 02:54:37 -0700 Subject: [PATCH] xl: Remove an unecessary lock with isBucketExist() (#2593) Fixes #2566 --- cmd/controller_test.go | 8 ++++---- cmd/lock-instrument.go | 2 +- cmd/lock-instrument_test.go | 8 ++++---- cmd/namespace-lock_test.go | 2 +- cmd/xl-v1-bucket.go | 17 ++--------------- 5 files changed, 12 insertions(+), 25 deletions(-) diff --git a/cmd/controller_test.go b/cmd/controller_test.go index ff3d1b2f0..477e1785d 100644 --- a/cmd/controller_test.go +++ b/cmd/controller_test.go @@ -88,7 +88,7 @@ func (s *TestRPCControllerSuite) TestRPCControlLock(c *C) { expectedVolPathBlockCount: 0, }, // Test case 2. - // Testing the existance of entry for the last read lock (read lock with opsID "9"). + // Testing the existence of entry for the last read lock (read lock with opsID "9"). { volume: "my-bucket", @@ -203,11 +203,11 @@ func (s *TestRPCControllerSuite) TestRPCControlLock(c *C) { // expected lock info. expectedLockStats := expectedResult[0] // verify the actual lock info with the expected one. - // verify the existance entry for first read lock (read lock with opsID "0"). + // verify the existence entry for first read lock (read lock with opsID "0"). verifyRPCLockInfoResponse(expectedLockStats, *reply, c, 1) expectedLockStats = expectedResult[1] // verify the actual lock info with the expected one. - // verify the existance entry for last read lock (read lock with opsID "9"). + // verify the existence entry for last read lock (read lock with opsID "9"). verifyRPCLockInfoResponse(expectedLockStats, *reply, c, 2) // now hold a write lock in a different go routine and it should block since 10 read locks are @@ -256,7 +256,7 @@ func (s *TestRPCControllerSuite) TestRPCControlLock(c *C) { nsMutex.RUnlock("my-bucket", "my-object", strconv.Itoa(i)) } wg.Wait() - // Since all the locks are released. There shouldnt be any entry in the lock info. + // Since all the locks are released. There should not be any entry in the lock info. // and all the counters should be set to 0. reply = &SystemLockState{} // Call the lock instrumentation RPC end point. diff --git a/cmd/lock-instrument.go b/cmd/lock-instrument.go index 474a7870d..5b0bd139e 100644 --- a/cmd/lock-instrument.go +++ b/cmd/lock-instrument.go @@ -255,7 +255,7 @@ func (n *nsLockMap) deleteLockInfoEntryForOps(param nsParam, operationID string) // decrease the global running and lock reference counter. n.runningLockCounter-- n.globalLockCounter-- - // decrease the lock referene counter for the lock info for given pair. + // decrease the lock referee counter for the lock info for given pair. // decrease the running operation number. Its assumed that the operation is over once an attempt to release the lock is made. infoMap.running-- // decrease the total reference count of locks jeld on pair. diff --git a/cmd/lock-instrument_test.go b/cmd/lock-instrument_test.go index d762ddfb2..a26b32815 100644 --- a/cmd/lock-instrument_test.go +++ b/cmd/lock-instrument_test.go @@ -34,7 +34,7 @@ type lockStateCase struct { expectedGlobalLockCount int // Total number of locks held across the system, includes blocked + held locks. expectedBlockedLockCount int // Total blocked lock across the system. - expectedRunningLockCount int // Total succesfully held locks (non-blocking). + expectedRunningLockCount int // Total successfully held locks (non-blocking). // expected lock statu for given pair. expectedVolPathLockCount int // Total locks held for given pair, includes blocked locks. expectedVolPathRunningCount int // Total succcesfully held locks for given pair. @@ -601,7 +601,7 @@ func TestNsLockMapDeleteLockInfoEntryForOps(t *testing.T) { } // Case - 3. - // Lock state is set to Running and then an attempt to delete the info for non-existant opsID done. + // Lock state is set to Running and then an attempt to delete the info for non-existent opsID done. nsMutex.lockMapMutex.Lock() err := nsMutex.statusNoneToBlocked(param, testCases[0].lockOrigin, testCases[0].opsID, testCases[0].readLock) if err != nil { @@ -612,9 +612,9 @@ func TestNsLockMapDeleteLockInfoEntryForOps(t *testing.T) { if err != nil { t.Fatalf("Setting lock status to Running failed: %s", err) } - actualErr = nsMutex.deleteLockInfoEntryForOps(param, "non-existant-OpsID") + actualErr = nsMutex.deleteLockInfoEntryForOps(param, "non-existent-OpsID") - expectedOpsIDErr := LockInfoOpsIDNotFound{param.volume, param.path, "non-existant-OpsID"} + expectedOpsIDErr := LockInfoOpsIDNotFound{param.volume, param.path, "non-existent-OpsID"} if actualErr != expectedOpsIDErr { t.Fatalf("Errors mismatch: Expected \"%s\", got \"%s\"", expectedOpsIDErr, actualErr) } diff --git a/cmd/namespace-lock_test.go b/cmd/namespace-lock_test.go index 8e91a3140..b2e359659 100644 --- a/cmd/namespace-lock_test.go +++ b/cmd/namespace-lock_test.go @@ -234,7 +234,7 @@ func TestLockStats(t *testing.T) { }, // Test case - 6. // Case where in the first 5 read locks are released, but 2 write locks are - // blocked waiting for the remaining 5 read locks locks to be released (10 read locks were held intially). + // blocked waiting for the remaining 5 read locks locks to be released (10 read locks were held initially). // We check the entry for the first blocked write call here. { diff --git a/cmd/xl-v1-bucket.go b/cmd/xl-v1-bucket.go index cc2c20602..55ee713ea 100644 --- a/cmd/xl-v1-bucket.go +++ b/cmd/xl-v1-bucket.go @@ -30,10 +30,6 @@ func (xl xlObjects) MakeBucket(bucket string) error { if !IsValidBucketName(bucket) { return BucketNameInvalid{Bucket: bucket} } - // Verify if bucket is found. - if xl.isBucketExist(bucket) { - return toObjectErr(errVolumeExists, bucket) - } // generates random string on setting MINIO_DEBUG=lock, else returns empty string. // used for instrumentation on locks. @@ -161,13 +157,6 @@ func (xl xlObjects) getBucketInfo(bucketName string) (bucketInfo BucketInfo, err // Checks whether bucket exists. func (xl xlObjects) isBucketExist(bucket string) bool { - // generates random string on setting MINIO_DEBUG=lock, else returns empty string. - // used for instrumentation on locks. - opsID := getOpsID() - - nsMutex.RLock(bucket, "", opsID) - defer nsMutex.RUnlock(bucket, "", opsID) - // Check whether bucket exists. _, err := xl.getBucketInfo(bucket) if err != nil { @@ -261,10 +250,6 @@ func (xl xlObjects) DeleteBucket(bucket string) error { if !IsValidBucketName(bucket) { return BucketNameInvalid{Bucket: bucket} } - // Verify if bucket is found. - if !xl.isBucketExist(bucket) { - return BucketNotFound{Bucket: bucket} - } // generates random string on setting MINIO_DEBUG=lock, else returns empty string. // used for instrumentation on locks. @@ -316,5 +301,7 @@ func (xl xlObjects) DeleteBucket(bucket string) error { }); reducedErr != nil { return toObjectErr(reducedErr, bucket) } + + // Success. return nil }