Repository: grpc/grpc-go

Issue:
Make least request available by default (remove GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST)
The gRFC https://github.com/grpc/proposal/blob/master/A48-xds-least-request-lb-policy.md states that the environment variable will be removed when the feature has proven stable. The last substantial changes to least request have been made in https://github.com/grpc/grpc-go/pull/6603 (1.58, Sep 2023).

We have successfully used least request in production for about 1 year, only discovering one minor issue last summer (fixed in https://github.com/grpc/grpc-go/pull/7467, 1.66).

We should consider removing the environment variable and always making least request available.

Discussion:
@arjan-bal: Based on the [comment from Eric](https://github.com/grpc/grpc-java/issues/11996#issuecomment-2787250309) assigning to @dfawley to share the result of the discussion.@davinci26: +1 on what @atollena is saying, we are also using it in `prod` for 6m+ with no issues@dfawley: #8253 is merged to enable it by default.  Let's leave this open to track removing the flag after the 1.73 release.

Solution patch:
diff --git a/internal/envconfig/envconfig.go b/internal/envconfig/envconfig.go
index f5f2bdeb..2fdaed88 100644
--- a/internal/envconfig/envconfig.go
+++ b/internal/envconfig/envconfig.go
@@ -33,10 +33,6 @@ var (
 	// "GRPC_RING_HASH_CAP".  This does not override the default bounds
 	// checking which NACKs configs specifying ring sizes > 8*1024*1024 (~8M).
 	RingHashCap = uint64FromEnv("GRPC_RING_HASH_CAP", 4096, 1, 8*1024*1024)
-	// LeastRequestLB is set if we should support the least_request_experimental
-	// LB policy, which can be enabled by setting the environment variable
-	// "GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST" to "true".
-	LeastRequestLB = boolFromEnv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST", true)
 	// ALTSMaxConcurrentHandshakes is the maximum number of concurrent ALTS
 	// handshakes that can be performed.
 	ALTSMaxConcurrentHandshakes = uint64FromEnv("GRPC_ALTS_MAX_CONCURRENT_HANDSHAKES", 100, 1, 100)
diff --git a/xds/internal/xdsclient/xdslbregistry/converter/converter.go b/xds/internal/xdsclient/xdslbregistry/converter/converter.go
index f0d064c6..959c6588 100644
--- a/xds/internal/xdsclient/xdslbregistry/converter/converter.go
+++ b/xds/internal/xdsclient/xdslbregistry/converter/converter.go
@@ -33,7 +33,6 @@ import (
 	"google.golang.org/grpc/balancer/ringhash"
 	"google.golang.org/grpc/balancer/roundrobin"
 	"google.golang.org/grpc/balancer/weightedroundrobin"
-	"google.golang.org/grpc/internal/envconfig"
 	iringhash "google.golang.org/grpc/internal/ringhash"
 	internalserviceconfig "google.golang.org/grpc/internal/serviceconfig"
 	"google.golang.org/grpc/xds/internal/balancer/wrrlocality"
@@ -177,9 +176,6 @@ func convertWeightedRoundRobinProtoToServiceConfig(rawProto []byte, _ int) (json
 }
 
 func convertLeastRequestProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) {
-	if !envconfig.LeastRequestLB {
-		return nil, nil
-	}
 	lrProto := &v3leastrequestpb.LeastRequest{}
 	if err := proto.Unmarshal(rawProto, lrProto); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal resource: %v", err)
diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_cds.go b/xds/internal/xdsclient/xdsresource/unmarshal_cds.go
index 00b73383..c2ced0d6 100644
--- a/xds/internal/xdsclient/xdsresource/unmarshal_cds.go
+++ b/xds/internal/xdsclient/xdsresource/unmarshal_cds.go
@@ -133,10 +133,6 @@ func validateClusterAndConstructClusterUpdate(cluster *v3clusterpb.Cluster, serv
 		rhLBCfg := []byte(fmt.Sprintf("{\"minRingSize\": %d, \"maxRingSize\": %d}", minSize, maxSize))
 		lbPolicy = []byte(fmt.Sprintf(`[{"ring_hash_experimental": %s}]`, rhLBCfg))
 	case v3clusterpb.Cluster_LEAST_REQUEST:
-		if !envconfig.LeastRequestLB {
-			return ClusterUpdate{}, fmt.Errorf("unexpected lbPolicy %v in response: %+v", cluster.GetLbPolicy(), cluster)
-		}
-
 		// "The configuration for the Least Request LB policy is the
 		// least_request_lb_config field. The field is optional; if not present,
 		// defaults will be assumed for all of its values." - A48



Test patch:
diff --git a/test/xds/xds_client_custom_lb_test.go b/test/xds/xds_client_custom_lb_test.go
index 2c881301..210e7e01 100644
--- a/test/xds/xds_client_custom_lb_test.go
+++ b/test/xds/xds_client_custom_lb_test.go
@@ -28,7 +28,6 @@ import (
 	_ "google.golang.org/grpc/balancer/leastrequest"       // To register least_request
 	_ "google.golang.org/grpc/balancer/weightedroundrobin" // To register weighted_round_robin
 	"google.golang.org/grpc/credentials/insecure"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/stubserver"
 	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/internal/testutils/roundrobin"
@@ -94,12 +93,6 @@ func clusterWithLBConfiguration(t *testing.T, clusterName, edsServiceName string
 // first) child load balancing policy, and asserts the correct distribution
 // based on the locality weights and the endpoint picking policy specified.
 func (s) TestWrrLocality(t *testing.T) {
-	oldLeastRequestLBSupport := envconfig.LeastRequestLB
-	envconfig.LeastRequestLB = true
-	defer func() {
-		envconfig.LeastRequestLB = oldLeastRequestLBSupport
-	}()
-
 	backend1 := stubserver.StartTestService(t, nil)
 	port1 := testutils.ParsePort(t, backend1.Address)
 	defer backend1.Stop()
diff --git a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go
index edb455b9..5760e632 100644
--- a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go
+++ b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go
@@ -27,7 +27,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	_ "google.golang.org/grpc/balancer/roundrobin"
 	"google.golang.org/grpc/internal/balancer/stub"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/grpctest"
 	"google.golang.org/grpc/internal/pretty"
 	internalserviceconfig "google.golang.org/grpc/internal/serviceconfig"
@@ -70,9 +69,6 @@ func wrrLocalityBalancerConfig(childPolicy *internalserviceconfig.BalancerConfig
 }
 
 func (s) TestConvertToServiceConfigSuccess(t *testing.T) {
-	defer func(old bool) { envconfig.LeastRequestLB = old }(envconfig.LeastRequestLB)
-	envconfig.LeastRequestLB = false
-
 	const customLBPolicyName = "myorg.MyCustomLeastRequestPolicy"
 	stub.Register(customLBPolicyName, stub.BalancerFuncs{})
 
@@ -198,26 +194,6 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) {
 			},
 			wantConfig: `[{"pick_first": { "shuffleAddressList": true }}]`,
 		},
-		{
-			name: "least_request_disabled_pf_rr_use_first_supported",
-			policy: &v3clusterpb.LoadBalancingPolicy{
-				Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{
-					{
-						TypedExtensionConfig: &v3corepb.TypedExtensionConfig{
-							TypedConfig: testutils.MarshalAny(t, &v3leastrequestpb.LeastRequest{
-								ChoiceCount: wrapperspb.UInt32(32),
-							}),
-						},
-					},
-					{
-						TypedExtensionConfig: &v3corepb.TypedExtensionConfig{
-							TypedConfig: testutils.MarshalAny(t, &v3roundrobinpb.RoundRobin{}),
-						},
-					},
-				},
-			},
-			wantConfig: `[{"round_robin": {}}]`,
-		},
 		{
 			name: "custom_lb_type_v3_struct",
 			policy: &v3clusterpb.LoadBalancingPolicy{
@@ -308,10 +284,6 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			if test.lrEnabled {
-				defer func(old bool) { envconfig.LeastRequestLB = old }(envconfig.LeastRequestLB)
-				envconfig.LeastRequestLB = true
-			}
 			rawJSON, err := xdslbregistry.ConvertToServiceConfig(test.policy, 0)
 			if err != nil {
 				t.Fatalf("ConvertToServiceConfig(%s) failed: %v", pretty.ToJSON(test.policy), err)
@@ -382,7 +354,7 @@ func (s) TestConvertToServiceConfigFailure(t *testing.T) {
 					},
 					{
 						TypedExtensionConfig: &v3corepb.TypedExtensionConfig{
-							// Not supported by gRPC-Go.
+							// Maglev is not yet supported by gRPC.
 							TypedConfig: testutils.MarshalAny(t, &v3maglevpb.Maglev{}),
 						},
 					},
diff --git a/xds/internal/xdsclient/xdsresource/tests/unmarshal_cds_test.go b/xds/internal/xdsclient/xdsresource/tests/unmarshal_cds_test.go
index f122b372..c9eaef5a 100644
--- a/xds/internal/xdsclient/xdsresource/tests/unmarshal_cds_test.go
+++ b/xds/internal/xdsclient/xdsresource/tests/unmarshal_cds_test.go
@@ -27,7 +27,6 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"google.golang.org/grpc/balancer/leastrequest"
 	"google.golang.org/grpc/internal/balancer/stub"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/grpctest"
 	iringhash "google.golang.org/grpc/internal/ringhash"
 	iserviceconfig "google.golang.org/grpc/internal/serviceconfig"
@@ -105,8 +104,6 @@ func (s) TestValidateCluster_Success(t *testing.T) {
 		t.Fatalf("Failed to create server config for testing: %v", err)
 	}
 
-	defer func(old bool) { envconfig.LeastRequestLB = old }(envconfig.LeastRequestLB)
-	envconfig.LeastRequestLB = true
 	tests := []struct {
 		name         string
 		cluster      *v3clusterpb.Cluster
diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go
index 33c70ecf..513241e0 100644
--- a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go
+++ b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go
@@ -107,7 +107,7 @@ func (s) TestValidateCluster_Failure(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "not-supported-lb-policy",
+			name: "unsupported-lb-policy",
 			cluster: &v3clusterpb.Cluster{
 				ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
 				EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{