config: remove retry disable via environment variable (#4922)

This commit is contained in:
Doug Fawley 2021-11-09 13:06:38 -08:00 committed by GitHub
parent 714ba8d517
commit c25a52b769
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 10 additions and 49 deletions

View File

@ -29,7 +29,6 @@ import (
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/internal"
internalbackoff "google.golang.org/grpc/internal/backoff"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/transport"
"google.golang.org/grpc/keepalive"
"google.golang.org/grpc/resolver"
@ -576,7 +575,6 @@ func withHealthCheckFunc(f internal.HealthChecker) DialOption {
func defaultDialOptions() dialOptions {
return dialOptions{
disableRetry: !envconfig.Retry,
healthCheckFunc: internal.HealthCheckFunc,
copts: transport.ConnectOptions{
WriteBufferSize: defaultWriteBufSize,

View File

@ -22,20 +22,14 @@ package envconfig
import (
"os"
"strings"
xdsenv "google.golang.org/grpc/internal/xds/env"
)
const (
prefix = "GRPC_GO_"
retryStr = prefix + "RETRY"
txtErrIgnoreStr = prefix + "IGNORE_TXT_ERRORS"
)
var (
// Retry is enabled unless explicitly disabled via "GRPC_GO_RETRY=off" or
// if XDS retry support is explicitly disabled.
Retry = !strings.EqualFold(os.Getenv(retryStr), "off") && xdsenv.RetrySupport
// TXTErrIgnore is set if TXT errors should be ignored ("GRPC_GO_IGNORE_TXT_ERRORS" is not "false").
TXTErrIgnore = !strings.EqualFold(os.Getenv(txtErrIgnoreStr), "false")
)

View File

@ -42,7 +42,6 @@ const (
ringHashSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH"
clientSideSecuritySupportEnv = "GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT"
aggregateAndDNSSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"
retrySupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY"
rbacSupportEnv = "GRPC_XDS_EXPERIMENTAL_RBAC"
federationEnv = "GRPC_EXPERIMENTAL_XDS_FEDERATION"
@ -81,9 +80,6 @@ var (
// "true".
AggregateAndDNSSupportEnv = strings.EqualFold(os.Getenv(aggregateAndDNSSupportEnv), "true")
// RetrySupport indicates whether xDS retry is enabled.
RetrySupport = !strings.EqualFold(os.Getenv(retrySupportEnv), "false")
// RBACSupport indicates whether xDS configured RBAC HTTP Filter is enabled,
// which can be disabled by setting the environment variable
// "GRPC_XDS_EXPERIMENTAL_RBAC" to "false".

View File

@ -33,7 +33,6 @@ import (
"github.com/golang/protobuf/proto"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/stubserver"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/stats"
@ -41,14 +40,7 @@ import (
testpb "google.golang.org/grpc/test/grpc_testing"
)
func enableRetry() func() {
old := envconfig.Retry
envconfig.Retry = true
return func() { envconfig.Retry = old }
}
func (s) TestRetryUnary(t *testing.T) {
defer enableRetry()()
i := -1
ss := &stubserver.StubServer{
EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) {
@ -116,7 +108,6 @@ func (s) TestRetryUnary(t *testing.T) {
}
func (s) TestRetryThrottling(t *testing.T) {
defer enableRetry()()
i := -1
ss := &stubserver.StubServer{
EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) {
@ -192,7 +183,6 @@ func (s) TestRetryThrottling(t *testing.T) {
}
func (s) TestRetryStreaming(t *testing.T) {
defer enableRetry()()
req := func(b byte) *testpb.StreamingOutputCallRequest {
return &testpb.StreamingOutputCallRequest{Payload: &testpb.Payload{Body: []byte{b}}}
}
@ -510,7 +500,6 @@ func (*retryStatsHandler) TagConn(ctx context.Context, _ *stats.ConnTagInfo) con
func (*retryStatsHandler) HandleConn(context.Context, stats.ConnStats) {}
func (s) TestRetryStats(t *testing.T) {
defer enableRetry()()
lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
t.Fatalf("Failed to listen. Err: %v", err)

View File

@ -32,7 +32,6 @@ import (
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/internal/stubserver"
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/status"
"google.golang.org/grpc/xds/internal/testutils/e2e"
@ -105,11 +104,6 @@ func (s) TestClientSideXDS(t *testing.T) {
}
func (s) TestClientSideRetry(t *testing.T) {
if !env.RetrySupport {
// Skip this test if retry is not enabled.
return
}
ctr := 0
errs := []codes.Code{codes.ResourceExhausted}
ss := &stubserver.StubServer{

View File

@ -109,7 +109,7 @@ func generateRDSUpdateFromRouteConfiguration(rc *v3routepb.RouteConfiguration, l
}
func generateRetryConfig(rp *v3routepb.RetryPolicy) (*RetryConfig, error) {
if !env.RetrySupport || rp == nil {
if rp == nil {
return nil, nil
}

View File

@ -99,10 +99,6 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
}
}
goodUpdateWithRetryPolicy = func(vhrc *RetryConfig, rrc *RetryConfig) RouteConfigUpdate {
if !env.RetrySupport {
vhrc = nil
rrc = nil
}
return RouteConfigUpdate{
VirtualHosts: []*VirtualHost{{
Domains: []string{ldsTarget},
@ -116,13 +112,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
}},
}
}
defaultRetryBackoff = RetryBackoff{BaseInterval: 25 * time.Millisecond, MaxInterval: 250 * time.Millisecond}
goodUpdateIfRetryDisabled = func() RouteConfigUpdate {
if env.RetrySupport {
return RouteConfigUpdate{}
}
return goodUpdateWithRetryPolicy(nil, nil)
}
defaultRetryBackoff = RetryBackoff{BaseInterval: 25 * time.Millisecond, MaxInterval: 250 * time.Millisecond}
)
tests := []struct {
@ -554,26 +544,26 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
{
name: "bad-retry-policy-0-retries",
rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", NumRetries: &wrapperspb.UInt32Value{Value: 0}}, nil),
wantUpdate: goodUpdateIfRetryDisabled(),
wantError: env.RetrySupport,
wantUpdate: RouteConfigUpdate{},
wantError: true,
},
{
name: "bad-retry-policy-0-base-interval",
rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{BaseInterval: durationpb.New(0)}}, nil),
wantUpdate: goodUpdateIfRetryDisabled(),
wantError: env.RetrySupport,
wantUpdate: RouteConfigUpdate{},
wantError: true,
},
{
name: "bad-retry-policy-negative-max-interval",
rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{MaxInterval: durationpb.New(-time.Second)}}, nil),
wantUpdate: goodUpdateIfRetryDisabled(),
wantError: env.RetrySupport,
wantUpdate: RouteConfigUpdate{},
wantError: true,
},
{
name: "bad-retry-policy-negative-max-interval-no-known-retry-on",
rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "something", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{MaxInterval: durationpb.New(-time.Second)}}, nil),
wantUpdate: goodUpdateIfRetryDisabled(),
wantError: env.RetrySupport,
wantUpdate: RouteConfigUpdate{},
wantError: true,
},
}
for _, test := range tests {