Skip to content

Commit 1f23f6c

Browse files
dfawleydfawley
authored
client: fix Connect to handle channel idleness properly (#6331)
1 parent 3ea58ce commit 1f23f6c

2 files changed

Lines changed: 52 additions & 12 deletions

File tree

‎clientconn.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,8 @@ func (cc *ClientConn) exitIdleMode() error {
325325
return errConnClosing
326326
}
327327
if cc.idlenessState != ccIdlenessStateIdle {
328-
logger.Error("ClientConn asked to exit idle mode when not in idle mode")
328+
cc.mu.Unlock()
329+
logger.Info("ClientConn asked to exit idle mode when not in idle mode")
329330
return nil
330331
}
331332

@@ -706,6 +707,9 @@ func (cc *ClientConn) GetState() connectivity.State {
706707
// Notice: This API is EXPERIMENTAL and may be changed or removed in a later
707708
// release.
708709
func (cc *ClientConn) Connect() {
710+
cc.exitIdleMode()
711+
// If the ClientConn was not in idle mode, we need to call ExitIdle on the
712+
// LB policy so that connections can be created.
709713
cc.balancerWrapper.exitIdleMode()
710714
}
711715

‎test/idleness_test.go‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,12 @@ func (s) TestChannelIdleness_Disabled_NoActivity(t *testing.T) {
107107
t.Cleanup(backend.Stop)
108108
r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backend.Address}}})
109109

110-
// Veirfy that the ClientConn moves to READY.
110+
// Verify that the ClientConn moves to READY.
111111
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
112112
defer cancel()
113113
awaitState(ctx, t, cc, connectivity.Ready)
114114

115-
// Veirfy that the ClientConn stay in READY.
115+
// Verify that the ClientConn stay in READY.
116116
sCtx, sCancel := context.WithTimeout(ctx, 3*defaultTestShortIdleTimeout)
117117
defer sCancel()
118118
awaitNoStateChange(sCtx, t, cc, connectivity.Ready)
@@ -152,12 +152,12 @@ func (s) TestChannelIdleness_Enabled_NoActivity(t *testing.T) {
152152
t.Cleanup(backend.Stop)
153153
r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backend.Address}}})
154154

155-
// Veirfy that the ClientConn moves to READY.
155+
// Verify that the ClientConn moves to READY.
156156
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
157157
defer cancel()
158158
awaitState(ctx, t, cc, connectivity.Ready)
159159

160-
// Veirfy that the ClientConn moves to IDLE as there is no activity.
160+
// Verify that the ClientConn moves to IDLE as there is no activity.
161161
awaitState(ctx, t, cc, connectivity.Idle)
162162

163163
// Verify idleness related channelz events.
@@ -203,7 +203,7 @@ func (s) TestChannelIdleness_Enabled_OngoingCall(t *testing.T) {
203203
t.Cleanup(backend.Stop)
204204
r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backend.Address}}})
205205

206-
// Veirfy that the ClientConn moves to READY.
206+
// Verify that the ClientConn moves to READY.
207207
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
208208
defer cancel()
209209
awaitState(ctx, t, cc, connectivity.Ready)
@@ -213,7 +213,7 @@ func (s) TestChannelIdleness_Enabled_OngoingCall(t *testing.T) {
213213
// the server RPC handler and the unary call below.
214214
errCh := make(chan error, 1)
215215
go func() {
216-
// Veirfy that the ClientConn stay in READY.
216+
// Verify that the ClientConn stay in READY.
217217
sCtx, sCancel := context.WithTimeout(ctx, 3*defaultTestShortIdleTimeout)
218218
defer sCancel()
219219
awaitNoStateChange(sCtx, t, cc, connectivity.Ready)
@@ -277,7 +277,7 @@ func (s) TestChannelIdleness_Enabled_ActiveSinceLastCheck(t *testing.T) {
277277
t.Cleanup(backend.Stop)
278278
r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backend.Address}}})
279279

280-
// Veirfy that the ClientConn moves to READY.
280+
// Verify that the ClientConn moves to READY.
281281
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
282282
defer cancel()
283283
awaitState(ctx, t, cc, connectivity.Ready)
@@ -302,7 +302,7 @@ func (s) TestChannelIdleness_Enabled_ActiveSinceLastCheck(t *testing.T) {
302302
}
303303
}()
304304

305-
// Veirfy that the ClientConn stay in READY.
305+
// Verify that the ClientConn stay in READY.
306306
awaitNoStateChange(sCtx, t, cc, connectivity.Ready)
307307

308308
// Verify that there are no idleness related channelz events.
@@ -343,12 +343,12 @@ func (s) TestChannelIdleness_Enabled_ExitIdleOnRPC(t *testing.T) {
343343
}
344344
t.Cleanup(func() { cc.Close() })
345345

346-
// Veirfy that the ClientConn moves to READY.
346+
// Verify that the ClientConn moves to READY.
347347
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
348348
defer cancel()
349349
awaitState(ctx, t, cc, connectivity.Ready)
350350

351-
// Veirfy that the ClientConn moves to IDLE as there is no activity.
351+
// Verify that the ClientConn moves to IDLE as there is no activity.
352352
awaitState(ctx, t, cc, connectivity.Idle)
353353

354354
// Verify idleness related channelz events.
@@ -405,7 +405,7 @@ func (s) TestChannelIdleness_Enabled_IdleTimeoutRacesWithRPCs(t *testing.T) {
405405
}
406406
t.Cleanup(func() { cc.Close() })
407407

408-
// Veirfy that the ClientConn moves to READY.
408+
// Verify that the ClientConn moves to READY.
409409
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
410410
defer cancel()
411411
awaitState(ctx, t, cc, connectivity.Ready)
@@ -421,3 +421,39 @@ func (s) TestChannelIdleness_Enabled_IdleTimeoutRacesWithRPCs(t *testing.T) {
421421
}
422422
}
423423
}
424+
425+
// Tests the case where the channel is IDLE and we call cc.Connect.
426+
func (s) TestChannelIdleness_Connect(t *testing.T) {
427+
// Start a test backend and set the bootstrap state of the resolver to
428+
// include this address. This will ensure that when the resolver is
429+
// restarted when exiting idle, it will push the same address to grpc again.
430+
r := manual.NewBuilderWithScheme("whatever")
431+
backend := stubserver.StartTestService(t, nil)
432+
t.Cleanup(backend.Stop)
433+
r.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: backend.Address}}})
434+
435+
// Create a ClientConn with a short idle_timeout.
436+
dopts := []grpc.DialOption{
437+
grpc.WithTransportCredentials(insecure.NewCredentials()),
438+
grpc.WithResolvers(r),
439+
grpc.WithIdleTimeout(defaultTestShortIdleTimeout),
440+
grpc.WithDefaultServiceConfig(`{"loadBalancingConfig": [{"round_robin":{}}]}`),
441+
}
442+
cc, err := grpc.Dial(r.Scheme()+":///test.server", dopts...)
443+
if err != nil {
444+
t.Fatalf("grpc.Dial() failed: %v", err)
445+
}
446+
t.Cleanup(func() { cc.Close() })
447+
448+
// Verify that the ClientConn moves to IDLE.
449+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
450+
defer cancel()
451+
452+
awaitState(ctx, t, cc, connectivity.Idle)
453+
454+
// Connect should exit channel idleness.
455+
cc.Connect()
456+
457+
// Verify that the ClientConn moves back to READY.
458+
awaitState(ctx, t, cc, connectivity.Ready)
459+
}

0 commit comments

Comments
 (0)