Merge pull request #20590 from ahrtr/20250901_lease_renew

Re-check that the lease still exists during the renew process

Author: ahrtr

server/lease/lessor.go

@@ -428,6 +428,8 @@ func (le *lessor) Renew(id LeaseID) (int64, error) {
 		}
 	}
 
+	// gofail: var beforeCheckpointInLeaseRenew struct{}
+
 	// Clear remaining TTL when we renew if it is set
 	// By applying a RAFT entry only when the remainingTTL is already set, we limit the number
 	// of RAFT entries written per lease to a max of 2 per checkpoint interval.
@@ -438,6 +440,12 @@ func (le *lessor) Renew(id LeaseID) (int64, error) {
 	}
 
 	le.mu.Lock()
+	// Re-check in case the lease was revoked immediately after the previous check
+	l = le.leaseMap[id]
+	if l == nil {
+		le.mu.Unlock()
+		return -1, ErrLeaseNotFound
+	}
 	l.refresh(0)
 	item := &LeaseWithTime{id: l.ID, time: l.expiry}
 	le.leaseExpiredNotifier.RegisterOrUpdate(item)

tests/e2e/v3_lease_no_proxy_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
 	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/tests/v3/framework/e2e"
 	"go.etcd.io/etcd/tests/v3/framework/testutils"
@@ -164,3 +165,60 @@ func testLeaseRevokeIssue(t *testing.T, clusterSize int, connectToOneFollower bo
 	close(stopC)
 	<-doneC
 }
+
+func TestLeaseRevokeDuringRenew(t *testing.T) {
+	e2e.BeforeTest(t)
+
+	ctx := t.Context()
+
+	t.Log("Starting a new etcd cluster")
+	epc, err := e2e.NewEtcdProcessCluster(ctx, t,
+		e2e.WithClusterSize(1),
+		e2e.WithGoFailEnabled(true),
+		e2e.WithGoFailClientTimeout(40*time.Second),
+	)
+	require.NoError(t, err)
+	defer func() {
+		require.NoErrorf(t, epc.Close(), "error closing etcd processes")
+	}()
+
+	eps := epc.Procs[0].EndpointsGRPC()
+	t.Logf("Creating two clients for lease operations: %v", eps)
+	clientForRenew, err := clientv3.New(clientv3.Config{Endpoints: eps, DialTimeout: 3 * time.Second})
+	require.NoError(t, err)
+	defer clientForRenew.Close()
+
+	clientForRevoke, err := clientv3.New(clientv3.Config{Endpoints: eps, DialTimeout: 3 * time.Second})
+	require.NoError(t, err)
+	defer clientForRevoke.Close()
+
+	t.Log("Creating a new lease")
+	leaseRsp, err := clientForRenew.Grant(ctx, 60)
+	require.NoError(t, err)
+
+	t.Log("Activate the 'beforeCheckpointInLeaseRenew' failpoint")
+	require.NoError(t, epc.Procs[0].Failpoints().SetupHTTP(ctx, "beforeCheckpointInLeaseRenew", `sleep("3s")`))
+
+	t.Logf("Starting a goroutine to keep alive the lease: %d", leaseRsp.ID)
+	doneC := make(chan struct{})
+	startC := make(chan struct{}, 1)
+	var renewError error
+	go func() {
+		defer close(doneC)
+		startC <- struct{}{}
+		_, renewError = clientForRenew.KeepAliveOnce(ctx, leaseRsp.ID)
+	}()
+
+	t.Log("Wait for the KeepAliveOnce goroutine to get started")
+	<-startC
+	time.Sleep(200 * time.Millisecond)
+
+	t.Logf("Revoke the lease: %d", leaseRsp.ID)
+	_, lerr := clientForRevoke.Revoke(ctx, leaseRsp.ID)
+	require.NoError(t, lerr)
+
+	t.Log("Waiting for the keepAlive goroutine to exit")
+	<-doneC
+
+	require.ErrorIs(t, rpctypes.ErrLeaseNotFound, renewError)
+}

tests/robustness/Makefile

@@ -86,15 +86,15 @@ install-gofail: $(GOPATH)/bin/gofail
 
 .PHONY: gofail-enable
 gofail-enable: $(GOPATH)/bin/gofail
-	$(GOPATH)/bin/gofail enable server/etcdserver/ server/lease/leasehttp server/storage/backend/ server/storage/mvcc/ server/storage/wal/ server/etcdserver/api/v3rpc/ server/etcdserver/api/membership/ server/etcdserver/api/rafthttp/
+	$(GOPATH)/bin/gofail enable server/etcdserver/ server/lease server/lease/leasehttp server/storage/backend/ server/storage/mvcc/ server/storage/wal/ server/etcdserver/api/v3rpc/ server/etcdserver/api/membership/ server/etcdserver/api/rafthttp/
 	cd $(REPOSITORY_ROOT)/server && go get go.etcd.io/gofail@${GOFAIL_VERSION}
 	cd $(REPOSITORY_ROOT)/etcdutl && go get go.etcd.io/gofail@${GOFAIL_VERSION}
 	cd $(REPOSITORY_ROOT)/etcdctl && go get go.etcd.io/gofail@${GOFAIL_VERSION}
 	cd $(REPOSITORY_ROOT)/tests && go get go.etcd.io/gofail@${GOFAIL_VERSION}
 
 .PHONY: gofail-disable
 gofail-disable: $(GOPATH)/bin/gofail
-	$(GOPATH)/bin/gofail disable server/etcdserver/ server/lease/leasehttp server/storage/backend/ server/storage/mvcc/ server/storage/wal/ server/etcdserver/api/v3rpc/ server/etcdserver/api/membership/ server/etcdserver/api/rafthttp/
+	$(GOPATH)/bin/gofail disable server/etcdserver/ server/lease server/lease/leasehttp server/storage/backend/ server/storage/mvcc/ server/storage/wal/ server/etcdserver/api/v3rpc/ server/etcdserver/api/membership/ server/etcdserver/api/rafthttp/
 	cd $(REPOSITORY_ROOT)/server && go mod tidy
 	cd $(REPOSITORY_ROOT)/etcdutl && go mod tidy
 	cd $(REPOSITORY_ROOT)/etcdctl && go mod tidy