Skip to content

Commit c8a85a3

Browse files
committed
dispatcher_test: remove use of deprecated UpdateSubConnState
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
1 parent d34c1d3 commit c8a85a3

File tree

2 files changed

+57
-36
lines changed

2 files changed

+57
-36
lines changed

broker/protocol/dispatcher.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (d *dispatcher) UpdateClientConnState(_ balancer.ClientConnState) error {
110110
// implements its own resolution and selection of an appropriate A record.
111111
func (d *dispatcher) ResolverError(_ error) {}
112112

113-
func (d *dispatcher) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) {
113+
func (d *dispatcher) updateSubConnState(sc balancer.SubConn, state balancer.SubConnState) {
114114
d.mu.Lock()
115115
var id, ok = d.connID[sc]
116116
if !ok {
@@ -141,6 +141,10 @@ func (d *dispatcher) UpdateSubConnState(sc balancer.SubConn, state balancer.SubC
141141
})
142142
}
143143

144+
func (d *dispatcher) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) {
145+
d.updateSubConnState(sc, state)
146+
}
147+
144148
// markedSubConn tracks the last mark associated with a SubConn.
145149
// SubConns not used for a complete sweep interval are closed.
146150
type markedSubConn struct {
@@ -176,7 +180,11 @@ func (d *dispatcher) Pick(info balancer.PickInfo) (balancer.PickResult, error) {
176180
[]resolver.Address{{
177181
Addr: d.idToAddr(dr.route, dispatchID),
178182
}},
179-
balancer.NewSubConnOptions{},
183+
balancer.NewSubConnOptions{
184+
StateListener: func(state balancer.SubConnState) {
185+
d.updateSubConnState(msc.subConn, state)
186+
},
187+
},
180188
); err != nil {
181189
return balancer.PickResult{}, err
182190
}

broker/protocol/dispatcher_test.go

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func (s *DispatcherSuite) TestContextAdapters(c *gc.C) {
4545
func (s *DispatcherSuite) TestDispatchCases(c *gc.C) {
4646
var cc mockClientConn
4747
var disp = dispatcherBuilder{zone: "local"}.Build(&cc, balancer.BuildOptions{}).(*dispatcher)
48+
cc.disp = disp
4849
close(disp.sweepDoneCh) // Disable async sweeping.
4950

5051
// Case: Called without a dispatchRoute. Expect it panics.
@@ -58,75 +59,75 @@ func (s *DispatcherSuite) TestDispatchCases(c *gc.C) {
5859
// SubConn to the default service address is started.
5960
var _, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
6061
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
61-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"default.addr"})
62+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name:"default.addr", disp: disp}})
6263
cc.created = nil
6364

6465
// Case: Default connection transitions to Ready. Expect it's now returned.
65-
disp.UpdateSubConnState(mockSubConn("default.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
66+
mockSubConn{Name: "default.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
6667

6768
result, err := disp.Pick(balancer.PickInfo{Ctx: ctx})
6869
c.Check(err, gc.IsNil)
6970
c.Check(result.Done, gc.IsNil)
70-
c.Check(result.SubConn, gc.Equals, mockSubConn("default.addr"))
71+
c.Check(result.SubConn, gc.Equals, mockSubConn{Name: "default.addr", disp: disp})
7172

7273
// Case: Specific remote peer is dispatched to.
7374
ctx = WithDispatchRoute(context.Background(),
7475
buildRouteFixture(), ProcessSpec_ID{Zone: "remote", Suffix: "primary"})
7576

7677
result, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
7778
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
78-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"remote.addr"})
79+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "remote.addr", disp: disp}})
7980
cc.created = nil
8081

81-
disp.UpdateSubConnState(mockSubConn("remote.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
82+
mockSubConn{Name:"remote.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
8283

8384
result, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
8485
c.Check(err, gc.IsNil)
8586
c.Check(result.Done, gc.IsNil)
86-
c.Check(result.SubConn, gc.Equals, mockSubConn("remote.addr"))
87+
c.Check(result.SubConn, gc.Equals, mockSubConn{Name: "remote.addr", disp: disp })
8788

8889
// Case: Route allows for multiple members. A local one is now dialed.
8990
ctx = WithDispatchRoute(context.Background(), buildRouteFixture(), ProcessSpec_ID{})
9091

9192
_, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
9293
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
93-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"local.addr"})
94+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name:"local.addr", disp: disp}})
9495
cc.created = nil
9596

96-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
97+
mockSubConn{Name:"local.addr",disp:disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
9798

9899
result, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
99100
c.Check(err, gc.IsNil)
100101
c.Check(result.Done, gc.IsNil)
101-
c.Check(result.SubConn, gc.Equals, mockSubConn("local.addr"))
102+
c.Check(result.SubConn, gc.Equals, mockSubConn{Name:"local.addr", disp: disp})
102103

103104
// Case: One local addr is marked as failed. Another is dialed.
104-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
105+
mockSubConn{Name: "local.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
105106

106107
_, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
107108
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
108-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"local.otherAddr"})
109+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "local.otherAddr", disp: disp}})
109110
cc.created = nil
110111

111-
disp.UpdateSubConnState(mockSubConn("local.otherAddr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
112+
mockSubConn{Name: "local.otherAddr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
112113

113114
result, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
114115
c.Check(err, gc.IsNil)
115116
c.Check(result.Done, gc.IsNil)
116-
c.Check(result.SubConn, gc.Equals, mockSubConn("local.otherAddr"))
117+
c.Check(result.SubConn, gc.Equals, mockSubConn{Name:"local.otherAddr", disp: disp})
117118

118119
// Case: otherAddr is also failed. Expect that an error is returned,
119120
// rather than dispatch to remote addr. (Eg we prefer to wait for a
120121
// local replica to recover or the route to change, vs using a remote
121122
// endpoint which incurs more networking cost).
122-
disp.UpdateSubConnState(mockSubConn("local.otherAddr"), balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
123+
mockSubConn{Name: "local.otherAddr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
123124

124125
_, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
125126
c.Check(err, gc.Equals, balancer.ErrTransientFailure)
126127

127128
// Case: local.addr is Ready again. However, primary is required and has failed.
128-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
129-
disp.UpdateSubConnState(mockSubConn("remote.addr"), balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
129+
mockSubConn{Name: "local.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
130+
mockSubConn{Name: "remote.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
130131

131132
ctx = WithDispatchRoute(context.Background(),
132133
buildRouteFixture(), ProcessSpec_ID{Zone: "remote", Suffix: "primary"})
@@ -150,7 +151,7 @@ func (s *DispatcherSuite) TestDispatchCases(c *gc.C) {
150151
result, err = disp.Pick(balancer.PickInfo{Ctx: ctx})
151152
c.Check(err, gc.IsNil)
152153
c.Check(result.Done, gc.NotNil)
153-
c.Check(result.SubConn, gc.Equals, mockSubConn("local.addr"))
154+
c.Check(result.SubConn, gc.Equals, mockSubConn{Name:"local.addr", disp: disp})
154155

155156
// Closure callback with an Unavailable error (only) will trigger an invalidation.
156157
result.Done(balancer.DoneInfo{Err: nil})
@@ -164,6 +165,7 @@ func (s *DispatcherSuite) TestDispatchCases(c *gc.C) {
164165
func (s *DispatcherSuite) TestDispatchMarkAndSweep(c *gc.C) {
165166
var cc mockClientConn
166167
var disp = dispatcherBuilder{zone: "local"}.Build(&cc, balancer.BuildOptions{}).(*dispatcher)
168+
cc.disp = disp
167169
defer disp.Close()
168170

169171
var err error
@@ -177,11 +179,11 @@ func (s *DispatcherSuite) TestDispatchMarkAndSweep(c *gc.C) {
177179
_, err = disp.Pick(balancer.PickInfo{Ctx: localCtx})
178180
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
179181

180-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"remote.addr", "local.addr"})
182+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "remote.addr", disp: disp}, mockSubConn{Name: "local.addr", disp: disp}})
181183
cc.created = nil
182184

183-
disp.UpdateSubConnState(mockSubConn("remote.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
184-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.Connecting})
185+
mockSubConn{Name: "remote.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
186+
mockSubConn{Name: "local.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Connecting})
185187

186188
disp.sweep()
187189
c.Check(cc.removed, gc.IsNil)
@@ -205,14 +207,14 @@ func (s *DispatcherSuite) TestDispatchMarkAndSweep(c *gc.C) {
205207

206208
// This time, expect that local.addr is swept.
207209
disp.sweep()
208-
c.Check(cc.removed, gc.DeepEquals, []mockSubConn{"local.addr"})
210+
c.Check(cc.removed, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "local.addr", disp: disp }})
209211
cc.removed = nil
210-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.Shutdown})
212+
mockSubConn{Name: "local.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Shutdown})
211213

212214
disp.sweep() // Now remote.addr is swept.
213-
c.Check(cc.removed, gc.DeepEquals, []mockSubConn{"remote.addr"})
215+
c.Check(cc.removed, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "remote.addr", disp: disp}})
214216
cc.removed = nil
215-
disp.UpdateSubConnState(mockSubConn("remote.addr"), balancer.SubConnState{ConnectivityState: connectivity.Shutdown})
217+
mockSubConn{Name: "remote.addr", disp: disp }.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Shutdown})
216218

217219
// No connections remain.
218220
c.Check(disp.idConn, gc.HasLen, 0)
@@ -223,10 +225,10 @@ func (s *DispatcherSuite) TestDispatchMarkAndSweep(c *gc.C) {
223225
_, err = disp.Pick(balancer.PickInfo{Ctx: localCtx})
224226
c.Check(err, gc.Equals, balancer.ErrNoSubConnAvailable)
225227

226-
c.Check(cc.created, gc.DeepEquals, []mockSubConn{"local.addr"})
228+
c.Check(cc.created, gc.DeepEquals, []mockSubConn{mockSubConn{Name: "local.addr", disp: disp}})
227229
cc.created = nil
228230

229-
disp.UpdateSubConnState(mockSubConn("local.addr"), balancer.SubConnState{ConnectivityState: connectivity.Ready})
231+
mockSubConn{Name: "local.addr", disp: disp}.UpdateState(balancer.SubConnState{ConnectivityState: connectivity.Ready})
230232
_, err = disp.Pick(balancer.PickInfo{Ctx: localCtx})
231233
c.Check(err, gc.IsNil)
232234
}
@@ -235,31 +237,42 @@ type mockClientConn struct {
235237
err error
236238
created []mockSubConn
237239
removed []mockSubConn
240+
disp *dispatcher
238241
}
239242

240-
type mockSubConn string
243+
type mockSubConn struct {
244+
Name string
245+
disp *dispatcher
246+
}
247+
248+
func (s1 mockSubConn) Equal(s2 mockSubConn) bool {
249+
return s1.Name == s2.Name
250+
}
241251

242-
func (s mockSubConn) UpdateAddresses([]resolver.Address) {}
252+
func (s mockSubConn) UpdateAddresses([]resolver.Address) { panic("deprecated") }
253+
func (s mockSubConn) UpdateState(state balancer.SubConnState) { s.disp.updateSubConnState(s, state) }
243254
func (s mockSubConn) Connect() {}
244255
func (s mockSubConn) GetOrBuildProducer(balancer.ProducerBuilder) (balancer.Producer, func()) {
245256
return nil, func() {}
246257
}
247-
func (s mockSubConn) Shutdown() {}
258+
func (s mockSubConn) Shutdown() {
259+
var c = s.disp.cc.(*mockClientConn)
260+
c.removed = append(c.removed, s)
261+
}
248262

249263
func (c *mockClientConn) NewSubConn(a []resolver.Address, _ balancer.NewSubConnOptions) (balancer.SubConn, error) {
250-
var sc = mockSubConn(a[0].Addr)
264+
var sc = mockSubConn{Name: a[0].Addr, disp: c.disp}
251265
c.created = append(c.created, sc)
252266
return sc, c.err
253267
}
254268

255-
func (c *mockClientConn) RemoveSubConn(sc balancer.SubConn) {
256-
c.removed = append(c.removed, sc.(mockSubConn))
257-
}
258-
259269
func (c *mockClientConn) UpdateAddresses(balancer.SubConn, []resolver.Address) {}
260270
func (c *mockClientConn) UpdateState(balancer.State) {}
261271
func (c *mockClientConn) ResolveNow(resolver.ResolveNowOptions) {}
262272
func (c *mockClientConn) Target() string { return "default.addr" }
273+
func (c *mockClientConn) RemoveSubConn(balancer.SubConn) {
274+
panic("deprecated")
275+
}
263276

264277
type mockRouter struct{ invalidated string }
265278

0 commit comments

Comments
 (0)