diff --git a/weed/cluster/lock_manager/distributed_lock_manager.go b/weed/cluster/lock_manager/distributed_lock_manager.go index 6c50e98563..699acd74ed 100644 --- a/weed/cluster/lock_manager/distributed_lock_manager.go +++ b/weed/cluster/lock_manager/distributed_lock_manager.go @@ -46,7 +46,7 @@ func (dlm *DistributedLockManager) LockWithTimeout(key string, expiredAtNs int64 // ring update and lock transfer completion — the old primary remains // authoritative for locks it still holds. if token != "" { - if lock, found := dlm.lockManager.GetLock(key); found && !lock.IsBackup && lock.Token == token { + if lock, found := dlm.lockManager.GetLock(key); found && !lock.IsBackup && (lock.Token == token || lock.PrevToken == token) { var seq int64 lockOwner, renewToken, generation, seq, err = dlm.lockManager.Lock(key, expiredAtNs, token, owner) if err == nil && renewToken != "" { @@ -95,7 +95,7 @@ func (dlm *DistributedLockManager) Unlock(key string, token string) (movedTo pb. } if primary != dlm.Host { // If we still hold this lock locally, serve the unlock here - if lock, found := dlm.lockManager.GetLock(key); found && !lock.IsBackup && lock.Token == token { + if lock, found := dlm.lockManager.GetLock(key); found && !lock.IsBackup && token != "" && (lock.Token == token || lock.PrevToken == token) { var isUnlocked bool var generation int64 var seq int64 diff --git a/weed/cluster/lock_manager/lock_manager.go b/weed/cluster/lock_manager/lock_manager.go index e5c4a9d985..e027254e7f 100644 --- a/weed/cluster/lock_manager/lock_manager.go +++ b/weed/cluster/lock_manager/lock_manager.go @@ -24,6 +24,7 @@ type LockManager struct { } type Lock struct { Token string + PrevToken string // token superseded by the latest renewal; a holder whose renewal response was lost still presents it ExpiredAtNs int64 Key string // only used for moving locks Owner string @@ -102,19 +103,28 @@ func (lm *LockManager) Lock(path string, expiredAtNs int64, token string, owner generation = oldValue.Generation // keep same generation on renewal seq = oldValue.Seq + 1 glog.V(4).Infof("key %s old token %v owner %v => %v owner %v", path, oldValue.Token, oldValue.Owner, renewToken, owner) - lm.locks[path] = &Lock{Token: renewToken, ExpiredAtNs: expiredAtNs, Owner: owner, Generation: generation, Seq: seq} - return - } else { - if token == "" { - // new lock - glog.V(4).Infof("key %s locked by %v", path, oldValue.Owner) - err = fmt.Errorf("lock already owned by %v", oldValue.Owner) - return - } - glog.V(4).Infof("key %s expected token %v owner %v received %v from %v", path, oldValue.Token, oldValue.Owner, token, owner) - err = fmt.Errorf("lock: token mismatch") + lm.locks[path] = &Lock{Token: renewToken, PrevToken: oldValue.Token, ExpiredAtNs: expiredAtNs, Owner: owner, Generation: generation, Seq: seq} return } + if token != "" && token == oldValue.PrevToken && owner == oldValue.Owner { + // The latest renewal already rotated the token but its response + // was lost: replay the current token so the holder recovers it + // instead of conflicting with its own lease until it expires. + renewToken = oldValue.Token + generation = oldValue.Generation + seq = oldValue.Seq + oldValue.ExpiredAtNs = expiredAtNs + return + } + if token == "" { + // new lock + glog.V(4).Infof("key %s locked by %v", path, oldValue.Owner) + err = fmt.Errorf("lock already owned by %v", oldValue.Owner) + return + } + glog.V(4).Infof("key %s expected token %v owner %v received %v from %v", path, oldValue.Token, oldValue.Owner, token, owner) + err = fmt.Errorf("lock: token mismatch") + return } else { glog.V(4).Infof("key %s no lock owner %v", path, owner) if token == "" { @@ -148,7 +158,7 @@ func (lm *LockManager) Unlock(path string, token string) (isUnlocked bool, gener delete(lm.locks, path) return } - if oldValue.Token == token { + if oldValue.Token == token || (token != "" && token == oldValue.PrevToken) { isUnlocked = true generation = oldValue.Generation seq = oldValue.Seq + 1 diff --git a/weed/cluster/lock_manager/lock_manager_test.go b/weed/cluster/lock_manager/lock_manager_test.go index 5caf31da29..d8101ed435 100644 --- a/weed/cluster/lock_manager/lock_manager_test.go +++ b/weed/cluster/lock_manager/lock_manager_test.go @@ -84,3 +84,39 @@ func TestLockManager_RemoveBackupLockRejectsOlderGeneration(t *testing.T) { assert.Equal(t, int64(8), lock.Generation) assert.Equal(t, int64(1), lock.Seq) } + +func TestLockManager_LostRenewalResponseReplaysToken(t *testing.T) { + lm := NewLockManager() + expires := time.Now().Add(30 * time.Second).UnixNano() + + _, token1, _, _, err := lm.Lock("key1", expires, "", "owner1") + assert.NoError(t, err) + + // The renewal rotated token1 to token2; the client never learned it. + _, token2, _, _, err := lm.Lock("key1", expires, token1, "owner1") + assert.NoError(t, err) + assert.NotEqual(t, token1, token2) + + // Retrying with the superseded token replays the current one instead of + // conflicting with its own lease until it expires. + owner, replayed, _, _, err := lm.Lock("key1", expires, token1, "owner1") + assert.NoError(t, err) + assert.Equal(t, "owner1", owner) + assert.Equal(t, token2, replayed) + + // A replay from a different owner still mismatches. + _, _, _, _, err = lm.Lock("key1", expires, token1, "intruder") + assert.Error(t, err) + + // The next rotation retires the superseded token for good. + _, token3, _, _, err := lm.Lock("key1", expires, token2, "owner1") + assert.NoError(t, err) + assert.NotEqual(t, token2, token3) + _, _, _, _, err = lm.Lock("key1", expires, token1, "owner1") + assert.Error(t, err) + + // A superseded token still releases its own lock. + unlocked, _, _, err := lm.Unlock("key1", token2) + assert.NoError(t, err) + assert.True(t, unlocked) +}