ringhash: allow overriding max ringhash size via environment variable

internal/envconfig/envconfig.go

@@ -21,6 +21,7 @@ package envconfig
 
 import (
 	"os"
+	"strconv"
 	"strings"
 )
 
@@ -37,3 +38,17 @@ var (
 	// ("GRPC_GO_ADVERTISE_COMPRESSORS" is not "false").
 	AdvertiseCompressors = !strings.EqualFold(os.Getenv(advertiseCompressorsStr), "false")
 )
+
+func uint64FromEnv(envVar string, def, min, max uint64) uint64 {
+	v, err := strconv.ParseUint(os.Getenv(envVar), 10, 64)
+	if err != nil {
+		return def
+	}
+	if v < min {
+		return min
+	}
+	if v > max {
+		return max
+	}
+	return v
+}

internal/envconfig/envconfig_test.go

@@ -0,0 +1,74 @@
+/*
+ *
+ * Copyright 2022 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package envconfig
+
+import (
+	"os"
+	"testing"
+
+	"google.golang.org/grpc/internal/grpctest"
+)
+
+type s struct {
+	grpctest.Tester
+}
+
+func Test(t *testing.T) {
+	grpctest.RunSubTests(t, s{})
+}
+
+func (s) TestUint64FromEnv(t *testing.T) {
+
+	var testCases = []struct {
+		name          string
+		val           string
+		def, min, max uint64
+		want          uint64
+	}{
+		{
+			name: "error parsing; want default",
+			val:  "asdf", def: 5, want: 5,
+		}, {
+			name: "unset; want default",
+			val:  "", def: 5, want: 5,
+		}, {
+			name: "too low; want min",
+			val:  "5", min: 10, want: 10,
+		}, {
+			name: "too high; want max",
+			val:  "5", max: 2, want: 2,
+		}, {
+			name: "in range; good",
+			val:  "17391", def: 13000, min: 12000, max: 18000, want: 17391,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			const testVar = "testvar"
+			if tc.val == "" {
+				os.Unsetenv(testVar)
+			} else {
+				os.Setenv(testVar, tc.val)
+			}
+			if got := uint64FromEnv(testVar, tc.def, tc.min, tc.max); got != tc.want {
+				t.Errorf("uint64FromEnv(%q(=%q), %v, %v, %v) = %v; want %v", testVar, tc.val, tc.def, tc.min, tc.max, got, tc.want)
+			}
+		})
+	}
+}

internal/envconfig/xds.go

@@ -99,3 +99,9 @@ var (
 	// C2PResolverTestOnlyTrafficDirectorURI is the TD URI for testing.
 	C2PResolverTestOnlyTrafficDirectorURI = os.Getenv(c2pResolverTestOnlyTrafficDirectorURIEnv)
 )
+
+// XDSRingHashLimit indicates the maximum ring size which defaults to 4096 but
+// may be overridden by setting the environment variable
+// "GRPC_XDS_RING_HASH_LIMIT".  A hard maximum limit of 8MB and minimum limit
+// of 1 is also enforced.
+var XDSRingHashLimit = uint64FromEnv("GRPC_XDS_RING_HASH_LIMIT", 4096, 1, 8*1024*1024)

xds/internal/balancer/ringhash/config.go

@@ -22,6 +22,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/serviceconfig"
 )
 
@@ -36,8 +37,6 @@ type LBConfig struct {
 const (
 	defaultMinSize = 1024
 	defaultMaxSize = 4096
-	// TODO(apolcyn): make makeRingSizeCap configurable, with either a dial option or global setting
-	maxRingSizeCap = 4096
 )
 
 func parseConfig(c json.RawMessage) (*LBConfig, error) {
@@ -51,14 +50,14 @@ func parseConfig(c json.RawMessage) (*LBConfig, error) {
 	if cfg.MaxRingSize == 0 {
 		cfg.MaxRingSize = defaultMaxSize
 	}
-	if cfg.MinRingSize > maxRingSizeCap {
-		cfg.MinRingSize = maxRingSizeCap
-	}
-	if cfg.MaxRingSize > maxRingSizeCap {
-		cfg.MaxRingSize = maxRingSizeCap
-	}
 	if cfg.MinRingSize > cfg.MaxRingSize {
 		return nil, fmt.Errorf("min %v is greater than max %v", cfg.MinRingSize, cfg.MaxRingSize)
 	}
+	if cfg.MinRingSize > envconfig.XDSRingHashLimit {
+		cfg.MinRingSize = envconfig.XDSRingHashLimit
+	}
+	if cfg.MaxRingSize > envconfig.XDSRingHashLimit {
+		cfg.MaxRingSize = envconfig.XDSRingHashLimit
+	}
 	return &cfg, nil
 }

xds/internal/balancer/ringhash/config_test.go

@@ -22,14 +22,16 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"google.golang.org/grpc/internal/envconfig"
 )
 
 func (s) TestParseConfig(t *testing.T) {
 	tests := []struct {
-		name    string
-		js      string
-		want    *LBConfig
-		wantErr bool
+		name           string
+		js             string
+		envConfigLimit uint64
+		want           *LBConfig
+		wantErr        bool
 	}{
 		{
 			name: "OK",
@@ -52,9 +54,42 @@ func (s) TestParseConfig(t *testing.T) {
 			want:    nil,
 			wantErr: true,
 		},
+		{
+			name:    "min greater than max greater than global limit",
+			js:      `{"minRingSize": 6000, "maxRingSize": 5000}`,
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "max greater than global limit",
+			js:   `{"minRingSize": 1, "maxRingSize": 6000}`,
+			want: &LBConfig{MinRingSize: 1, MaxRingSize: 4096},
+		},
+		{
+			name: "min and max greater than global limit",
+			js:   `{"minRingSize": 5000, "maxRingSize": 6000}`,
+			want: &LBConfig{MinRingSize: 4096, MaxRingSize: 4096},
+		},
+		{
+			name:           "min and max less than raised global limit",
+			js:             `{"minRingSize": 5000, "maxRingSize": 6000}`,
+			envConfigLimit: 8000,
+			want:           &LBConfig{MinRingSize: 5000, MaxRingSize: 6000},
+		},
+		{
+			name:           "min and max greater than raised global limit",
+			js:             `{"minRingSize": 10000, "maxRingSize": 10000}`,
+			envConfigLimit: 8000,
+			want:           &LBConfig{MinRingSize: 8000, MaxRingSize: 8000},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if tt.envConfigLimit != 0 {
+				old := envconfig.XDSRingHashLimit
+				defer func() { envconfig.XDSRingHashLimit = old }()
+				envconfig.XDSRingHashLimit = tt.envConfigLimit
+			}
 			got, err := parseConfig([]byte(tt.js))
 			if (err != nil) != tt.wantErr {
 				t.Errorf("parseConfig() error = %v, wantErr %v", err, tt.wantErr)