From bf0d7d8af8f25a0491e1fb9541fd5530e9f29908 Mon Sep 17 00:00:00 2001 From: boyce Date: Thu, 18 Apr 2024 11:30:19 +0800 Subject: [PATCH 1/4] add ErrNoSubConnAvailable reason to pick ctx timeout --- picker_wrapper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/picker_wrapper.go b/picker_wrapper.go index bf56faa76d3d..c0e568c607cf 100644 --- a/picker_wrapper.go +++ b/picker_wrapper.go @@ -151,6 +151,7 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. pickResult, err := p.Pick(info) if err != nil { if err == balancer.ErrNoSubConnAvailable { + lastPickErr = err continue } if st, ok := status.FromError(err); ok { From 7d14dd025c4a4da24862ab3a630cc7e03c480b6b Mon Sep 17 00:00:00 2001 From: boyce Date: Fri, 19 Apr 2024 17:56:56 +0800 Subject: [PATCH 2/4] add new picker test for NoSubConnAvailable --- picker_wrapper_test.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/picker_wrapper_test.go b/picker_wrapper_test.go index ba99a06b0f8c..7010bf426f24 100644 --- a/picker_wrapper_test.go +++ b/picker_wrapper_test.go @@ -26,6 +26,7 @@ import ( "time" "google.golang.org/grpc/balancer" + "google.golang.org/grpc/balancer/base" "google.golang.org/grpc/codes" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/internal/transport" @@ -69,11 +70,29 @@ func (s) TestBlockingPickTimeout(t *testing.T) { bp := newPickerWrapper(nil) ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) defer cancel() - if _, _, err := bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { + var err error + if _, _, err = bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { t.Errorf("bp.pick returned error %v, want DeadlineExceeded", err) } } +func (s) TestBlockingPickErrNoSubConnAvailableTimeout(t *testing.T) { + bp := newPickerWrapper(nil) + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) + defer cancel() + + bp.updatePicker(base.NewErrPicker(balancer.ErrNoSubConnAvailable)) + var err error + if _, _, err = bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { + t.Errorf("bp.pick returned error %v, want DeadlineExceeded", err) + } + + serr, _ := status.FromError(err) + if serr.Message() != "latest balancer error: "+balancer.ErrNoSubConnAvailable.Error() { + t.Errorf("bp.pick returned error %v, want balancer.ErrNoSubConnAvailable", err) + } +} + func (s) TestBlockingPick(t *testing.T) { bp := newPickerWrapper(nil) // All goroutines should block because picker is nil in bp. From dc6f6b6e2064967ccaae76c6c0e9aa8331e988e3 Mon Sep 17 00:00:00 2001 From: boyce Date: Fri, 19 Apr 2024 17:58:51 +0800 Subject: [PATCH 3/4] correct the incorrect edits of picker test --- picker_wrapper_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/picker_wrapper_test.go b/picker_wrapper_test.go index 7010bf426f24..7efd0bab257f 100644 --- a/picker_wrapper_test.go +++ b/picker_wrapper_test.go @@ -70,8 +70,7 @@ func (s) TestBlockingPickTimeout(t *testing.T) { bp := newPickerWrapper(nil) ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) defer cancel() - var err error - if _, _, err = bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { + if _, _, err := bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { t.Errorf("bp.pick returned error %v, want DeadlineExceeded", err) } } From 23f53442330ef3080d36dbb9ee164b136cf8b8a1 Mon Sep 17 00:00:00 2001 From: boyce Date: Tue, 7 May 2024 10:58:23 +0800 Subject: [PATCH 4/4] add user-friendly error message of LB policy update timed out --- picker_wrapper.go | 4 ++-- picker_wrapper_test.go | 18 ------------------ 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/picker_wrapper.go b/picker_wrapper.go index c0e568c607cf..56e8aba783f7 100644 --- a/picker_wrapper.go +++ b/picker_wrapper.go @@ -20,6 +20,7 @@ package grpc import ( "context" + "fmt" "io" "sync" @@ -117,7 +118,7 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. if lastPickErr != nil { errStr = "latest balancer error: " + lastPickErr.Error() } else { - errStr = ctx.Err().Error() + errStr = fmt.Sprintf("received context error while waiting for new LB policy update: %s", ctx.Err().Error()) } switch ctx.Err() { case context.DeadlineExceeded: @@ -151,7 +152,6 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. pickResult, err := p.Pick(info) if err != nil { if err == balancer.ErrNoSubConnAvailable { - lastPickErr = err continue } if st, ok := status.FromError(err); ok { diff --git a/picker_wrapper_test.go b/picker_wrapper_test.go index 7efd0bab257f..ba99a06b0f8c 100644 --- a/picker_wrapper_test.go +++ b/picker_wrapper_test.go @@ -26,7 +26,6 @@ import ( "time" "google.golang.org/grpc/balancer" - "google.golang.org/grpc/balancer/base" "google.golang.org/grpc/codes" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/internal/transport" @@ -75,23 +74,6 @@ func (s) TestBlockingPickTimeout(t *testing.T) { } } -func (s) TestBlockingPickErrNoSubConnAvailableTimeout(t *testing.T) { - bp := newPickerWrapper(nil) - ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) - defer cancel() - - bp.updatePicker(base.NewErrPicker(balancer.ErrNoSubConnAvailable)) - var err error - if _, _, err = bp.pick(ctx, true, balancer.PickInfo{}); status.Code(err) != codes.DeadlineExceeded { - t.Errorf("bp.pick returned error %v, want DeadlineExceeded", err) - } - - serr, _ := status.FromError(err) - if serr.Message() != "latest balancer error: "+balancer.ErrNoSubConnAvailable.Error() { - t.Errorf("bp.pick returned error %v, want balancer.ErrNoSubConnAvailable", err) - } -} - func (s) TestBlockingPick(t *testing.T) { bp := newPickerWrapper(nil) // All goroutines should block because picker is nil in bp.