Skip to content

Commit d5db534

Browse files
committed
fix precision of time cache and usedAt
1 parent b862bf5 commit d5db534

File tree

2 files changed

+263
-5
lines changed

2 files changed

+263
-5
lines changed

internal/pool/conn.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,9 @@ func NewConn(netConn net.Conn) *Conn {
131131
}
132132

133133
func NewConnWithBufferSize(netConn net.Conn, readBufSize, writeBufSize int) *Conn {
134+
now := time.Now()
134135
cn := &Conn{
135-
createdAt: time.Now(),
136+
createdAt: now,
136137
id: generateConnID(), // Generate unique ID for this connection
137138
stateMachine: NewConnStateMachine(),
138139
}
@@ -154,7 +155,7 @@ func NewConnWithBufferSize(netConn net.Conn, readBufSize, writeBufSize int) *Con
154155
cn.netConnAtomic.Store(&atomicNetConn{conn: netConn})
155156

156157
cn.wr = proto.NewWriter(cn.bw)
157-
cn.SetUsedAt(time.Now())
158+
cn.SetUsedAt(now)
158159
// Initialize handoff state atomically
159160
initialHandoffState := &HandoffState{
160161
ShouldHandoff: false,
@@ -166,12 +167,12 @@ func NewConnWithBufferSize(netConn net.Conn, readBufSize, writeBufSize int) *Con
166167
}
167168

168169
func (cn *Conn) UsedAt() time.Time {
169-
unix := atomic.LoadInt64(&cn.usedAt)
170-
return time.Unix(unix, 0)
170+
unixNano := atomic.LoadInt64(&cn.usedAt)
171+
return time.Unix(0, unixNano)
171172
}
172173

173174
func (cn *Conn) SetUsedAt(tm time.Time) {
174-
atomic.StoreInt64(&cn.usedAt, tm.Unix())
175+
atomic.StoreInt64(&cn.usedAt, tm.UnixNano())
175176
}
176177

177178
// Backward-compatible wrapper methods for state machine

internal/pool/conn_used_at_test.go

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
package pool
2+
3+
import (
4+
"context"
5+
"net"
6+
"testing"
7+
"time"
8+
9+
"github.com/redis/go-redis/v9/internal/proto"
10+
)
11+
12+
// TestConn_UsedAtUpdatedOnRead verifies that usedAt is updated when reading from connection
13+
func TestConn_UsedAtUpdatedOnRead(t *testing.T) {
14+
// Create a mock connection
15+
server, client := net.Pipe()
16+
defer server.Close()
17+
defer client.Close()
18+
19+
cn := NewConn(client)
20+
defer cn.Close()
21+
22+
// Get initial usedAt time
23+
initialUsedAt := cn.UsedAt()
24+
25+
// Wait at least 100ms to ensure time difference (usedAt has ~50ms precision from cached time)
26+
time.Sleep(100 * time.Millisecond)
27+
28+
// Simulate a read operation by calling WithReader
29+
ctx := context.Background()
30+
err := cn.WithReader(ctx, time.Second, func(rd *proto.Reader) error {
31+
// Don't actually read anything, just trigger the deadline update
32+
return nil
33+
})
34+
35+
if err != nil {
36+
t.Fatalf("WithReader failed: %v", err)
37+
}
38+
39+
// Get updated usedAt time
40+
updatedUsedAt := cn.UsedAt()
41+
42+
// Verify that usedAt was updated
43+
if !updatedUsedAt.After(initialUsedAt) {
44+
t.Errorf("Expected usedAt to be updated after read. Initial: %v, Updated: %v",
45+
initialUsedAt, updatedUsedAt)
46+
}
47+
48+
// Verify the difference is reasonable (should be around 100ms, accounting for ~50ms cache precision)
49+
diff := updatedUsedAt.Sub(initialUsedAt)
50+
if diff < 50*time.Millisecond || diff > 200*time.Millisecond {
51+
t.Errorf("Expected usedAt difference to be around 100ms (±50ms for cache), got %v", diff)
52+
}
53+
}
54+
55+
// TestConn_UsedAtUpdatedOnWrite verifies that usedAt is updated when writing to connection
56+
func TestConn_UsedAtUpdatedOnWrite(t *testing.T) {
57+
// Create a mock connection
58+
server, client := net.Pipe()
59+
defer server.Close()
60+
defer client.Close()
61+
62+
cn := NewConn(client)
63+
defer cn.Close()
64+
65+
// Get initial usedAt time
66+
initialUsedAt := cn.UsedAt()
67+
68+
// Wait at least 100ms to ensure time difference (usedAt has ~50ms precision from cached time)
69+
time.Sleep(100 * time.Millisecond)
70+
71+
// Simulate a write operation by calling WithWriter
72+
ctx := context.Background()
73+
err := cn.WithWriter(ctx, time.Second, func(wr *proto.Writer) error {
74+
// Don't actually write anything, just trigger the deadline update
75+
return nil
76+
})
77+
78+
if err != nil {
79+
t.Fatalf("WithWriter failed: %v", err)
80+
}
81+
82+
// Get updated usedAt time
83+
updatedUsedAt := cn.UsedAt()
84+
85+
// Verify that usedAt was updated
86+
if !updatedUsedAt.After(initialUsedAt) {
87+
t.Errorf("Expected usedAt to be updated after write. Initial: %v, Updated: %v",
88+
initialUsedAt, updatedUsedAt)
89+
}
90+
91+
// Verify the difference is reasonable (should be around 100ms, accounting for ~50ms cache precision)
92+
diff := updatedUsedAt.Sub(initialUsedAt)
93+
if diff < 50*time.Millisecond || diff > 200*time.Millisecond {
94+
t.Errorf("Expected usedAt difference to be around 100ms (±50ms for cache), got %v", diff)
95+
}
96+
}
97+
98+
// TestConn_UsedAtUpdatedOnMultipleOperations verifies that usedAt is updated on each operation
99+
func TestConn_UsedAtUpdatedOnMultipleOperations(t *testing.T) {
100+
// Create a mock connection
101+
server, client := net.Pipe()
102+
defer server.Close()
103+
defer client.Close()
104+
105+
cn := NewConn(client)
106+
defer cn.Close()
107+
108+
ctx := context.Background()
109+
var previousUsedAt time.Time
110+
111+
// Perform multiple operations and verify usedAt is updated each time
112+
// Note: usedAt has ~50ms precision from cached time
113+
for i := 0; i < 5; i++ {
114+
currentUsedAt := cn.UsedAt()
115+
116+
if i > 0 {
117+
// Verify usedAt was updated from previous iteration
118+
if !currentUsedAt.After(previousUsedAt) {
119+
t.Errorf("Iteration %d: Expected usedAt to be updated. Previous: %v, Current: %v",
120+
i, previousUsedAt, currentUsedAt)
121+
}
122+
}
123+
124+
previousUsedAt = currentUsedAt
125+
126+
// Wait at least 100ms (accounting for ~50ms cache precision)
127+
time.Sleep(100 * time.Millisecond)
128+
129+
// Perform a read operation
130+
err := cn.WithReader(ctx, time.Second, func(rd *proto.Reader) error {
131+
return nil
132+
})
133+
if err != nil {
134+
t.Fatalf("Iteration %d: WithReader failed: %v", i, err)
135+
}
136+
}
137+
138+
// Verify final usedAt is significantly later than initial
139+
finalUsedAt := cn.UsedAt()
140+
if !finalUsedAt.After(previousUsedAt) {
141+
t.Errorf("Expected final usedAt to be updated. Previous: %v, Final: %v",
142+
previousUsedAt, finalUsedAt)
143+
}
144+
}
145+
146+
// TestConn_UsedAtNotUpdatedWithoutOperation verifies that usedAt is NOT updated without operations
147+
func TestConn_UsedAtNotUpdatedWithoutOperation(t *testing.T) {
148+
// Create a mock connection
149+
server, client := net.Pipe()
150+
defer server.Close()
151+
defer client.Close()
152+
153+
cn := NewConn(client)
154+
defer cn.Close()
155+
156+
// Get initial usedAt time
157+
initialUsedAt := cn.UsedAt()
158+
159+
// Wait without performing any operations
160+
time.Sleep(100 * time.Millisecond)
161+
162+
// Get usedAt time again
163+
currentUsedAt := cn.UsedAt()
164+
165+
// Verify that usedAt was NOT updated (should be the same)
166+
if !currentUsedAt.Equal(initialUsedAt) {
167+
t.Errorf("Expected usedAt to remain unchanged without operations. Initial: %v, Current: %v",
168+
initialUsedAt, currentUsedAt)
169+
}
170+
}
171+
172+
// TestConn_UsedAtConcurrentUpdates verifies that usedAt updates are thread-safe
173+
func TestConn_UsedAtConcurrentUpdates(t *testing.T) {
174+
// Create a mock connection
175+
server, client := net.Pipe()
176+
defer server.Close()
177+
defer client.Close()
178+
179+
cn := NewConn(client)
180+
defer cn.Close()
181+
182+
ctx := context.Background()
183+
const numGoroutines = 10
184+
const numIterations = 10
185+
186+
// Launch multiple goroutines that perform operations concurrently
187+
done := make(chan bool, numGoroutines)
188+
for i := 0; i < numGoroutines; i++ {
189+
go func() {
190+
for j := 0; j < numIterations; j++ {
191+
// Alternate between read and write operations
192+
if j%2 == 0 {
193+
_ = cn.WithReader(ctx, time.Second, func(rd *proto.Reader) error {
194+
return nil
195+
})
196+
} else {
197+
_ = cn.WithWriter(ctx, time.Second, func(wr *proto.Writer) error {
198+
return nil
199+
})
200+
}
201+
time.Sleep(time.Millisecond)
202+
}
203+
done <- true
204+
}()
205+
}
206+
207+
// Wait for all goroutines to complete
208+
for i := 0; i < numGoroutines; i++ {
209+
<-done
210+
}
211+
212+
// Verify that usedAt was updated (should be recent)
213+
usedAt := cn.UsedAt()
214+
timeSinceUsed := time.Since(usedAt)
215+
216+
// Should be very recent (within last second)
217+
if timeSinceUsed > time.Second {
218+
t.Errorf("Expected usedAt to be recent, but it was %v ago", timeSinceUsed)
219+
}
220+
}
221+
222+
// TestConn_UsedAtPrecision verifies that usedAt has 50ms precision (not nanosecond)
223+
func TestConn_UsedAtPrecision(t *testing.T) {
224+
// Create a mock connection
225+
server, client := net.Pipe()
226+
defer server.Close()
227+
defer client.Close()
228+
229+
cn := NewConn(client)
230+
defer cn.Close()
231+
232+
ctx := context.Background()
233+
234+
// Perform an operation
235+
err := cn.WithReader(ctx, time.Second, func(rd *proto.Reader) error {
236+
return nil
237+
})
238+
if err != nil {
239+
t.Fatalf("WithReader failed: %v", err)
240+
}
241+
242+
// Get usedAt time
243+
usedAt := cn.UsedAt()
244+
245+
// Verify that usedAt has nanosecond precision (from the cached time which updates every 50ms)
246+
// The value should be reasonable (not year 1970 or something)
247+
if usedAt.Year() < 2020 {
248+
t.Errorf("Expected usedAt to be a recent time, got %v", usedAt)
249+
}
250+
251+
// The nanoseconds might be non-zero depending on when the cache was updated
252+
// We just verify the time is stored with full precision (not truncated to seconds)
253+
initialNanos := usedAt.UnixNano()
254+
if initialNanos == 0 {
255+
t.Error("Expected usedAt to have nanosecond precision, got 0")
256+
}
257+
}

0 commit comments

Comments
 (0)