Skip to content

Commit 0f1da99

Browse files
committed
Responded to Doug's comment and implemented offline discussion
1 parent 36b856c commit 0f1da99

File tree

2 files changed

+7
-16
lines changed

2 files changed

+7
-16
lines changed

balancer/rls/picker.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -254,19 +254,10 @@ func (p *rlsPicker) handleRouteLookupResponse(cacheKey cacheKey, targets []strin
254254
now := time.Now()
255255

256256
// "An RLS request is considered to have failed if it returns a non-OK
257-
// status or the RLS response's targets list does not contain any non-empty
258-
// entries." - RLS LB Policy design.
259-
allTargetsEmpty := true
260-
for _, target := range targets {
261-
if target != "" {
262-
allTargetsEmpty = false
263-
break
264-
}
265-
}
266-
if allTargetsEmpty {
267-
if err == nil {
268-
err = fmt.Errorf("RLS response's target list does not contain any non-empty entries for key %+v", cacheKey)
269-
}
257+
// status or the RLS response's targets list is non-empty." - RLS LB Policy
258+
// design.
259+
if len(targets) == 0 && err == nil {
260+
err = fmt.Errorf("RLS response's target list does not contain any entries for key %+v", cacheKey)
270261
// If err is set, rpc error from the control plane and no control plane
271262
// configuration is why no targets were passed into this helper, no need
272263
// to specify and tell the user this information.

balancer/rls/picker_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (s) TestNoNonEmptyTargetsReturnsError(t *testing.T) {
4949
// Setup RLS Server to return a response with an empty target string.
5050
rlsServer, rlsReqCh := rlstest.SetupFakeRLSServer(t, nil)
5151
rlsServer.SetResponseCallback(func(_ context.Context, req *rlspb.RouteLookupRequest) *rlstest.RouteLookupResponse {
52-
return &rlstest.RouteLookupResponse{Resp: &rlspb.RouteLookupResponse{Targets: []string{""}}}
52+
return &rlstest.RouteLookupResponse{Resp: &rlspb.RouteLookupResponse{}}
5353
})
5454

5555
// Register a manual resolver and push the RLS service config through it.
@@ -67,7 +67,7 @@ func (s) TestNoNonEmptyTargetsReturnsError(t *testing.T) {
6767
// target list does not contain any non empty entries.
6868
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
6969
defer cancel()
70-
makeTestRPCAndVerifyError(ctx, t, cc, codes.Unavailable, errors.New("RLS response's target list does not contain any non-empty entries for key"))
70+
makeTestRPCAndVerifyError(ctx, t, cc, codes.Unavailable, errors.New("RLS response's target list does not contain any entries for key"))
7171

7272
// Make sure an RLS request is sent out. Even though the RLS Server will
7373
// return no targets, the request should still hit the server.
@@ -161,7 +161,7 @@ func (s) TestPick_DataCacheMiss_NoPendingEntry_NotThrottled(t *testing.T) {
161161
// smaller timeout to ensure that the test doesn't run very long.
162162
ctx, cancel := context.WithTimeout(context.Background(), defaultTestShortTimeout)
163163
defer cancel()
164-
makeTestRPCAndVerifyError(ctx, t, cc, codes.Unavailable, errors.New("RLS response's target list does not contain any non-empty entries for key"))
164+
makeTestRPCAndVerifyError(ctx, t, cc, codes.Unavailable, errors.New("RLS response's target list does not contain any entries for key"))
165165

166166
// Make sure an RLS request is sent out.
167167
verifyRLSRequest(t, rlsReqCh, true)

0 commit comments

Comments
 (0)