Skip to content

Commit e3e142d

Browse files
arjan-balarjan-bal
authored
pickfirst: Remove old pickfirst (#8672)
Fixes: #8561 Addresses: #6472 The new pickfirst has been the default since [gRPC Go v1.71.0](https://github.com/grpc/grpc-go/releases/tag/v1.71.0) and all reported bugs have been fixed. This PR removes the old pickfirst policy completely. The exported symbols in the `pickfirstleaf` package are retained with a deprecation notice for removal after one release. RELEASE NOTES: * pickfirst: Remove the old `pick_first` LB policy. The new `pick_first` has been the default since v 1.71.0.
1 parent 254ab10 commit e3e142d

File tree

23 files changed

+2834
-3228
lines changed

23 files changed

+2834
-3228
lines changed

‎.github/workflows/coverage.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ jobs:
1919
- name: Run coverage
2020
run: go test -coverprofile=coverage.out -coverpkg=./... ./...
2121

22-
- name: Run coverage with old pickfirst
23-
run: GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST=false go test -coverprofile=coverage_old_pickfirst.out -coverpkg=./... ./...
24-
2522
- name: Upload coverage to Codecov
2623
uses: codecov/codecov-action@v4
2724
with:

‎.github/workflows/testing.yml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,6 @@ jobs:
6969
- type: tests
7070
goversion: '1.24'
7171

72-
- type: tests
73-
goversion: '1.25'
74-
testflags: -race
75-
grpcenv: 'GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST=false'
76-
7772
steps:
7873
# Setup the environment.
7974
- name: Setup GOARCH

‎balancer/endpointsharding/endpointsharding_ext_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
"google.golang.org/grpc/backoff"
3232
"google.golang.org/grpc/balancer"
3333
"google.golang.org/grpc/balancer/endpointsharding"
34-
"google.golang.org/grpc/balancer/pickfirst/pickfirstleaf"
34+
"google.golang.org/grpc/balancer/pickfirst"
3535
"google.golang.org/grpc/codes"
3636
"google.golang.org/grpc/connectivity"
3737
"google.golang.org/grpc/credentials/insecure"
@@ -84,7 +84,7 @@ func (fakePetioleBuilder) Build(cc balancer.ClientConn, opts balancer.BuildOptio
8484
ClientConn: cc,
8585
bOpts: opts,
8686
}
87-
fp.Balancer = endpointsharding.NewBalancer(fp, opts, balancer.Get(pickfirstleaf.Name).Build, endpointsharding.Options{})
87+
fp.Balancer = endpointsharding.NewBalancer(fp, opts, balancer.Get(pickfirst.Name).Build, endpointsharding.Options{})
8888
return fp
8989
}
9090

@@ -222,7 +222,7 @@ func (s) TestEndpointShardingReconnectDisabled(t *testing.T) {
222222
bf := stub.BalancerFuncs{
223223
Init: func(bd *stub.BalancerData) {
224224
epOpts := endpointsharding.Options{DisableAutoReconnect: true}
225-
bd.ChildBalancer = endpointsharding.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build, epOpts)
225+
bd.ChildBalancer = endpointsharding.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirst.Name).Build, epOpts)
226226
},
227227
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
228228
return bd.ChildBalancer.UpdateClientConnState(ccs)
@@ -303,7 +303,7 @@ func (s) TestEndpointShardingExitIdle(t *testing.T) {
303303
bf := stub.BalancerFuncs{
304304
Init: func(bd *stub.BalancerData) {
305305
epOpts := endpointsharding.Options{DisableAutoReconnect: true}
306-
bd.ChildBalancer = endpointsharding.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build, epOpts)
306+
bd.ChildBalancer = endpointsharding.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirst.Name).Build, epOpts)
307307
},
308308
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
309309
return bd.ChildBalancer.UpdateClientConnState(ccs)

‎balancer/lazy/lazy_ext_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
"google.golang.org/grpc"
3030
"google.golang.org/grpc/balancer"
3131
"google.golang.org/grpc/balancer/lazy"
32-
"google.golang.org/grpc/balancer/pickfirst/pickfirstleaf"
32+
"google.golang.org/grpc/balancer/pickfirst"
3333
"google.golang.org/grpc/connectivity"
3434
"google.golang.org/grpc/credentials/insecure"
3535
"google.golang.org/grpc/internal/balancer/stub"
@@ -79,7 +79,7 @@ func (s) TestExitIdle(t *testing.T) {
7979

8080
bf := stub.BalancerFuncs{
8181
Init: func(bd *stub.BalancerData) {
82-
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build)
82+
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirst.Name).Build)
8383
},
8484
ExitIdle: func(bd *stub.BalancerData) {
8585
bd.ChildBalancer.ExitIdle()
@@ -144,7 +144,7 @@ func (s) TestPicker(t *testing.T) {
144144

145145
bf := stub.BalancerFuncs{
146146
Init: func(bd *stub.BalancerData) {
147-
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build)
147+
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirst.Name).Build)
148148
},
149149
ExitIdle: func(*stub.BalancerData) {
150150
t.Log("Ignoring call to ExitIdle, calling the picker should make the lazy balancer exit IDLE state.")
@@ -201,7 +201,7 @@ func (s) TestGoodUpdateThenResolverError(t *testing.T) {
201201

202202
childBF := stub.BalancerFuncs{
203203
Init: func(bd *stub.BalancerData) {
204-
bd.ChildBalancer = balancer.Get(pickfirstleaf.Name).Build(bd.ClientConn, bd.BuildOptions)
204+
bd.ChildBalancer = balancer.Get(pickfirst.Name).Build(bd.ClientConn, bd.BuildOptions)
205205
},
206206
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
207207
if resolverErrorReceived.HasFired() {
@@ -306,7 +306,7 @@ func (s) TestResolverErrorThenGoodUpdate(t *testing.T) {
306306

307307
childBF := stub.BalancerFuncs{
308308
Init: func(bd *stub.BalancerData) {
309-
bd.ChildBalancer = balancer.Get(pickfirstleaf.Name).Build(bd.ClientConn, bd.BuildOptions)
309+
bd.ChildBalancer = balancer.Get(pickfirst.Name).Build(bd.ClientConn, bd.BuildOptions)
310310
},
311311
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
312312
return bd.ChildBalancer.UpdateClientConnState(ccs)
@@ -407,7 +407,7 @@ func (s) TestExitIdlePassthrough(t *testing.T) {
407407

408408
bf := stub.BalancerFuncs{
409409
Init: func(bd *stub.BalancerData) {
410-
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build)
410+
bd.ChildBalancer = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirst.Name).Build)
411411
},
412412
ExitIdle: func(bd *stub.BalancerData) {
413413
bd.ChildBalancer.ExitIdle()

‎balancer/leastrequest/leastrequest.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828

2929
"google.golang.org/grpc/balancer"
3030
"google.golang.org/grpc/balancer/endpointsharding"
31-
"google.golang.org/grpc/balancer/pickfirst/pickfirstleaf"
31+
"google.golang.org/grpc/balancer/pickfirst"
3232
"google.golang.org/grpc/connectivity"
3333
"google.golang.org/grpc/grpclog"
3434
internalgrpclog "google.golang.org/grpc/internal/grpclog"
@@ -90,7 +90,7 @@ func (bb) Build(cc balancer.ClientConn, bOpts balancer.BuildOptions) balancer.Ba
9090
ClientConn: cc,
9191
endpointRPCCounts: resolver.NewEndpointMap[*atomic.Int32](),
9292
}
93-
b.child = endpointsharding.NewBalancer(b, bOpts, balancer.Get(pickfirstleaf.Name).Build, endpointsharding.Options{})
93+
b.child = endpointsharding.NewBalancer(b, bOpts, balancer.Get(pickfirst.Name).Build, endpointsharding.Options{})
9494
b.logger = internalgrpclog.NewPrefixLogger(logger, fmt.Sprintf("[%p] ", b))
9595
b.logger.Infof("Created")
9696
return b
@@ -141,7 +141,7 @@ func (lrb *leastRequestBalancer) UpdateClientConnState(ccs balancer.ClientConnSt
141141
return lrb.child.UpdateClientConnState(balancer.ClientConnState{
142142
// Enable the health listener in pickfirst children for client side health
143143
// checks and outlier detection, if configured.
144-
ResolverState: pickfirstleaf.EnableHealthListener(ccs.ResolverState),
144+
ResolverState: pickfirst.EnableHealthListener(ccs.ResolverState),
145145
})
146146
}
147147

‎balancer/pickfirst/pickfirstleaf/metrics_test.go‎ renamed to ‎balancer/pickfirst/metrics_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616
*
1717
*/
1818

19-
package pickfirstleaf_test
19+
package pickfirst_test
2020

2121
import (
2222
"context"
2323
"fmt"
2424
"testing"
2525

2626
"google.golang.org/grpc"
27-
"google.golang.org/grpc/balancer/pickfirst/pickfirstleaf"
27+
"google.golang.org/grpc/balancer/pickfirst"
2828
"google.golang.org/grpc/connectivity"
2929
"google.golang.org/grpc/credentials/insecure"
3030
"google.golang.org/grpc/internal"
@@ -54,7 +54,7 @@ func init() {
5454
}
5555
}
5656
]
57-
}`, pickfirstleaf.Name)
57+
}`, pickfirst.Name)
5858
}
5959

6060
// TestPickFirstMetrics tests pick first metrics. It configures a pick first

0 commit comments

Comments
 (0)