Skip to content

Commit 7b5df49

Browse files
qts0312qts0312
committed
fix lease grant timeout
Signed-off-by: qts0312 <[email protected]>
1 parent cf5a571 commit 7b5df49

File tree

2 files changed

+225
-2
lines changed

2 files changed

+225
-2
lines changed

‎client/v3/concurrency/session.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ import (
2323
v3 "go.etcd.io/etcd/client/v3"
2424
)
2525

26-
const defaultSessionTTL = 60
26+
const (
27+
defaultSessionTTL = 60
28+
defaultLeaseTimeout = 5 * time.Second
29+
)
2730

2831
// Session represents a lease kept alive for the lifetime of a client.
2932
// Fault-tolerant applications may use sessions to reason about liveness.
@@ -47,7 +50,9 @@ func NewSession(client *v3.Client, opts ...SessionOption) (*Session, error) {
4750

4851
id := ops.leaseID
4952
if id == v3.NoLease {
50-
resp, err := client.Grant(ops.ctx, int64(ops.ttl))
53+
ctx, cancel := context.WithTimeout(ops.ctx, defaultLeaseTimeout)
54+
defer cancel()
55+
resp, err := client.Grant(ctx, int64(ops.ttl))
5156
if err != nil {
5257
return nil, err
5358
}

‎tests/integration/clientv3/concurrency/session_timeout_test.go‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
// Copyright 2025 The etcd Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//go:build !cluster_proxy
16+
17+
package concurrency_test
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
"go.etcd.io/etcd/client/v3/concurrency"
25+
"go.etcd.io/etcd/tests/v3/framework/integration"
26+
)
27+
28+
// TestNewSessionLeaseGrantTimeout tests that NewSession respects timeout when creating a lease
29+
func TestNewSessionLeaseGrantTimeout(t *testing.T) {
30+
integration.BeforeTest(t)
31+
32+
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
33+
defer clus.Terminate(t)
34+
35+
cli := clus.RandClient()
36+
37+
// Test case 1: Very short timeout should fail
38+
t.Run("ShortTimeoutShouldFail", func(t *testing.T) {
39+
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond)
40+
defer cancel()
41+
42+
start := time.Now()
43+
session, err := concurrency.NewSession(cli, concurrency.WithContext(ctx), concurrency.WithTTL(5))
44+
elapsed := time.Since(start)
45+
46+
if err == nil {
47+
if session != nil {
48+
session.Close()
49+
}
50+
t.Fatal("expected timeout error, but got nil")
51+
}
52+
53+
if err != context.DeadlineExceeded {
54+
t.Fatalf("expected context.DeadlineExceeded, got %v", err)
55+
}
56+
57+
// Should fail quickly (within reasonable time)
58+
if elapsed > 100*time.Millisecond {
59+
t.Fatalf("timeout took too long: %v", elapsed)
60+
}
61+
})
62+
63+
// Test case 2: Adequate timeout should succeed
64+
t.Run("AdequateTimeoutShouldSucceed", func(t *testing.T) {
65+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
66+
defer cancel()
67+
68+
session, err := concurrency.NewSession(cli, concurrency.WithContext(ctx), concurrency.WithTTL(5))
69+
if err != nil {
70+
t.Fatalf("expected success, got error: %v", err)
71+
}
72+
if session == nil {
73+
t.Fatal("expected valid session, got nil")
74+
}
75+
defer session.Close()
76+
77+
// Verify session has a valid lease
78+
if session.Lease() == 0 {
79+
t.Fatal("session should have a valid lease ID")
80+
}
81+
})
82+
}
83+
84+
// TestNewSessionTimeoutConsistency tests that timeout behavior is consistent with Close()
85+
func TestNewSessionTimeoutConsistency(t *testing.T) {
86+
integration.BeforeTest(t)
87+
88+
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
89+
defer clus.Terminate(t)
90+
91+
cli := clus.RandClient()
92+
93+
// Both NewSession and Close should use the same timeout pattern
94+
// This test verifies they behave consistently
95+
t.Run("TimeoutPatternConsistency", func(t *testing.T) {
96+
// Create session with short TTL
97+
session, err := concurrency.NewSession(cli, concurrency.WithTTL(1))
98+
if err != nil {
99+
t.Fatalf("failed to create session: %v", err)
100+
}
101+
102+
// Both LeaseGrant (in NewSession) and LeaseRevoke (in Close)
103+
// should use the same timeout duration (TTL seconds)
104+
start := time.Now()
105+
err = session.Close()
106+
elapsed := time.Since(start)
107+
108+
// Close should complete within reasonable time for TTL=1
109+
if elapsed > 3*time.Second {
110+
t.Fatalf("Close() took too long: %v, expected < 3s for TTL=1", elapsed)
111+
}
112+
113+
if err != nil {
114+
t.Logf("Close() returned error (acceptable): %v", err)
115+
}
116+
})
117+
}
118+
119+
// TestNewSessionNormalOperationAfterFix tests that normal session creation still works
120+
func TestNewSessionNormalOperationAfterFix(t *testing.T) {
121+
integration.BeforeTest(t)
122+
123+
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
124+
defer clus.Terminate(t)
125+
126+
cli := clus.RandClient()
127+
128+
// Verify normal session operations work correctly after the timeout fix
129+
session, err := concurrency.NewSession(cli, concurrency.WithTTL(60))
130+
if err != nil {
131+
t.Fatalf("failed to create session: %v", err)
132+
}
133+
defer session.Close()
134+
135+
// Session should be immediately usable
136+
if session.Lease() == 0 {
137+
t.Fatal("session should have a valid lease")
138+
}
139+
140+
// Keep-alive should be working
141+
select {
142+
case <-session.Done():
143+
t.Fatal("session should not be done immediately")
144+
case <-time.After(100 * time.Millisecond):
145+
// Expected - session should remain alive
146+
}
147+
148+
// Should be able to use session for mutex
149+
mutex := concurrency.NewMutex(session, "test-mutex")
150+
if mutex == nil {
151+
t.Fatal("should be able to create mutex with session")
152+
}
153+
}
154+
155+
// TestNewSessionTimeoutWithDifferentTTL tests timeout behavior with various TTL values
156+
func TestNewSessionTimeoutWithDifferentTTL(t *testing.T) {
157+
integration.BeforeTest(t)
158+
159+
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
160+
defer clus.Terminate(t)
161+
162+
cli := clus.RandClient()
163+
164+
testCases := []struct {
165+
name string
166+
ttl int
167+
contextTimeout time.Duration
168+
shouldTimeout bool
169+
}{
170+
{
171+
name: "short_ttl_short_timeout",
172+
ttl: 1,
173+
contextTimeout: 1 * time.Millisecond,
174+
shouldTimeout: true,
175+
},
176+
{
177+
name: "short_ttl_adequate_timeout",
178+
ttl: 1,
179+
contextTimeout: 3 * time.Second,
180+
shouldTimeout: false,
181+
},
182+
{
183+
name: "normal_ttl_adequate_timeout",
184+
ttl: 60,
185+
contextTimeout: 5 * time.Second,
186+
shouldTimeout: false,
187+
},
188+
}
189+
190+
for _, tc := range testCases {
191+
t.Run(tc.name, func(t *testing.T) {
192+
ctx, cancel := context.WithTimeout(context.Background(), tc.contextTimeout)
193+
defer cancel()
194+
195+
session, err := concurrency.NewSession(cli, concurrency.WithContext(ctx), concurrency.WithTTL(tc.ttl))
196+
197+
if tc.shouldTimeout {
198+
if err == nil {
199+
if session != nil {
200+
session.Close()
201+
}
202+
t.Fatal("expected timeout error, but got nil")
203+
}
204+
if err != context.DeadlineExceeded {
205+
t.Fatalf("expected context.DeadlineExceeded, got %v", err)
206+
}
207+
} else {
208+
if err != nil {
209+
t.Fatalf("expected no error, got %v", err)
210+
}
211+
if session == nil {
212+
t.Fatal("expected valid session, got nil")
213+
}
214+
session.Close()
215+
}
216+
})
217+
}
218+
}

0 commit comments

Comments
 (0)