Skip to content

Commit

Permalink
config: remove retry disable via environment variables
Browse files Browse the repository at this point in the history
  • Loading branch information
dfawley committed Nov 9, 2021
1 parent 14ebd91 commit cf6c6ec
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 49 deletions.
2 changes: 0 additions & 2 deletions dialoptions.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions internal/envconfig/envconfig.go
Expand Up @@ -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")
)
4 changes: 0 additions & 4 deletions internal/xds/env/env.go
Expand Up @@ -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"

Expand Down Expand Up @@ -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".
Expand Down
11 changes: 0 additions & 11 deletions test/retry_test.go
Expand Up @@ -33,22 +33,14 @@ 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"
"google.golang.org/grpc/status"
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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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}}}
}
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions xds/internal/test/xds_client_integration_test.go
Expand Up @@ -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"

Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion xds/internal/xdsclient/xdsresource/unmarshal_rds.go
Expand Up @@ -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
}

Expand Down
28 changes: 9 additions & 19 deletions xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go
Expand Up @@ -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},
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit cf6c6ec

Please sign in to comment.