Skip to content

Commit

Permalink
Merge pull request etcd-io#17058 from ahrtr/rollback_lease_lock_20231204
Browse files Browse the repository at this point in the history
Rollback the pull/14080: which fixed the potential deadlock
  • Loading branch information
ahrtr authored Dec 5, 2023
2 parents d8f0771 + 9a6eeb5 commit 4ece556
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions server/lease/lessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,17 +281,17 @@ func (le *lessor) Grant(id LeaseID, ttl int64) (*Lease, error) {
// with longer TTL to reduce renew load.
l := NewLease(id, ttl)

if l.ttl < le.minLeaseTTL {
l.ttl = le.minLeaseTTL
}

le.mu.Lock()
defer le.mu.Unlock()

if _, ok := le.leaseMap[id]; ok {
return nil, ErrLeaseExists
}

if l.ttl < le.minLeaseTTL {
l.ttl = le.minLeaseTTL
}

if le.isPrimary() {
l.refresh(0)
} else {
Expand Down Expand Up @@ -322,11 +322,6 @@ func (le *lessor) Revoke(id LeaseID) error {
return ErrLeaseNotFound
}

// We shouldn't delete the lease inside the transaction lock, otherwise
// it may lead to deadlock with Grant or Checkpoint operations, which
// acquire the le.mu firstly and then the batchTx lock.
delete(le.leaseMap, id)

defer close(l.revokec)
// unlock before doing external work
le.mu.Unlock()
Expand All @@ -345,6 +340,9 @@ func (le *lessor) Revoke(id LeaseID) error {
txn.DeleteRange([]byte(key), nil)
}

le.mu.Lock()
defer le.mu.Unlock()
delete(le.leaseMap, l.ID)
// lease deletion needs to be in the same backend transaction with the
// kv deletion. Or we might end up with not executing the revoke or not
// deleting the keys if etcdserver fails in between.
Expand Down

0 comments on commit 4ece556

Please sign in to comment.