diff --git a/cmd/lock-rest-server-common.go b/cmd/lock-rest-server-common.go index fc8059291..ce2d74c56 100644 --- a/cmd/lock-rest-server-common.go +++ b/cmd/lock-rest-server-common.go @@ -17,12 +17,8 @@ package cmd import ( - "context" - "errors" "path" "time" - - "github.com/minio/minio/cmd/logger" ) const lockRESTVersion = "v1" @@ -53,28 +49,22 @@ type lockResponse struct { func (l *localLocker) removeEntryIfExists(nlrip nameLockRequesterInfoPair) { // Check if entry is still in map (could have been removed altogether by 'concurrent' (R)Unlock of last entry) if lri, ok := l.lockMap[nlrip.name]; ok { - if !l.removeEntry(nlrip.name, nlrip.lri.UID, &lri) { - // Remove failed, in case it is a: - if nlrip.lri.Writer { - // Writer: this should never happen as the whole (mapped) entry should have been deleted - reqInfo := (&logger.ReqInfo{}).AppendTags("name", nlrip.name) - reqInfo.AppendTags("uid", nlrip.lri.UID) - ctx := logger.SetReqInfo(context.Background(), reqInfo) - logger.LogIf(ctx, errors.New("Lock maintenance failed to remove entry for write lock (should never happen)")) - } // Reader: this can happen if multiple read locks were active and - // the one we are looking for has been released concurrently (so it is fine). - } // Removal went okay, all is fine. + // Even if the entry exists, it may not be the same entry which was + // considered as expired, so we simply an attempt to remove it if its + // not possible there is nothing we need to do. + l.removeEntry(nlrip.name, nlrip.lri.UID, &lri) } } -// removeEntry either, based on the uid of the lock message, removes a single entry from the -// lockRequesterInfo array or the whole array from the map (in case of a write lock or last read lock) +// removeEntry based on the uid of the lock message, removes a single entry from the +// lockRequesterInfo array or the whole array from the map (in case of a write lock +// or last read lock) func (l *localLocker) removeEntry(name, uid string, lri *[]lockRequesterInfo) bool { // Find correct entry to remove based on uid. for index, entry := range *lri { if entry.UID == uid { if len(*lri) == 1 { - // Remove the (last) lock. + // Remove the write lock. delete(l.lockMap, name) } else { // Remove the appropriate read lock. @@ -84,6 +74,7 @@ func (l *localLocker) removeEntry(name, uid string, lri *[]lockRequesterInfo) bo return true } } + // None found return false, perhaps entry removed in previous run. return false } diff --git a/cmd/lock-rest-server.go b/cmd/lock-rest-server.go index ada0ef47e..80c77aa69 100644 --- a/cmd/lock-rest-server.go +++ b/cmd/lock-rest-server.go @@ -286,17 +286,17 @@ func (l *lockRESTServer) lockMaintenance(interval time.Duration) { Resource: nlrip.name, }) - // Close the connection regardless of the call response. - c.Close() - - // For successful response, verify if lock is indeed active or stale. + // For successful response, verify if lock was indeed active or stale. if expired { - // The lock is no longer active at server that originated the lock - // So remove the lock from the map. + // The lock is no longer active at server that originated + // the lock, attempt to remove the lock. l.ll.mutex.Lock() l.ll.removeEntryIfExists(nlrip) // Purge the stale entry if it exists. l.ll.mutex.Unlock() } + + // Close the connection regardless of the call response. + c.Close() } }