Skip to content

Commit 4a6103d

Browse files
authored
feat(bigtable): prime initial channel pool connections in parallel (#20010)
## Summary - `NewBigtableChannelPool` previously primed each connection serially, so a pool of N connections waited `N * (dial + Prime + retry-backoff)` before returning. With Prime() on the order of tens of ms per connection, that dominates client startup time and scales linearly with pool size. - This PR fans the prime calls out across an `errgroup` with `SetLimit(min(N, 10))`. The cap is intentional: very large pools should not spawn one dial+Prime goroutine per connection; small pools naturally use `connPoolSize` workers. - Failure semantics are preserved: the first prime error cancels the rest via the derived context, and the existing cleanup loop closes any populated entries before returning the error. - The Direct Access "firstConn" path still seats the primed probe connection at index 0 and skips a worker for that slot. ## Test plan - [x] `go build ./...` - [x] `go vet ./...` - [x] `go test -race -timeout 240s ./bigtable/internal/transport/...` - [x] New `TestNewBigtableChannelPoolParallelPriming` covers pool sizes below, at, and above the 10-worker cap. Asserts peak concurrent in-flight `PingAndWarm` matches expected worker count, and wall time beats the sequential bound (`N * primeDelay`). - [x] `fakeService` now tracks peak in-flight `PingAndWarm` so the test can assert concurrency without timing-only signals. - [x] `TestNewBigtableChannelPool/DialFailure` updated to use `atomic.Int64` for its counter (the dial function is now called concurrently).
1 parent 21c4a44 commit 4a6103d

4 files changed

Lines changed: 128 additions & 35 deletions

File tree

bigtable/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ require (
2323
go.opentelemetry.io/otel/sdk v1.44.0
2424
go.opentelemetry.io/otel/sdk/metric v1.44.0
2525
golang.org/x/oauth2 v0.36.0
26+
golang.org/x/sync v0.20.0
2627
google.golang.org/api v0.283.0
2728
google.golang.org/genproto v0.0.0-20260519071638-aa98bba5eb94
2829
google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa
@@ -55,7 +56,6 @@ require (
5556
go.opentelemetry.io/otel/trace v1.44.0 // indirect
5657
golang.org/x/crypto v0.51.0 // indirect
5758
golang.org/x/net v0.55.0 // indirect
58-
golang.org/x/sync v0.20.0 // indirect
5959
golang.org/x/sys v0.45.0 // indirect
6060
golang.org/x/text v0.37.0 // indirect
6161
golang.org/x/time v0.15.0 // indirect

bigtable/internal/transport/connpool.go

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package internal
1616

1717
import (
1818
"context"
19-
"errors"
2019
"fmt"
2120
"log"
2221
"math"
@@ -36,6 +35,7 @@ import (
3635
"go.opentelemetry.io/otel/attribute"
3736
"go.opentelemetry.io/otel/metric"
3837
"golang.org/x/oauth2/google"
38+
"golang.org/x/sync/errgroup"
3939
gtransport "google.golang.org/api/transport/grpc"
4040
"google.golang.org/grpc/codes"
4141
"google.golang.org/grpc/credentials/alts"
@@ -61,6 +61,11 @@ const (
6161
artificialLoadIfError = 10
6262
artificialLoadPenalizedTimer = 5 * time.Second
6363
requestParamsHeader = "x-goog-request-params"
64+
// maxPrimeWorkers caps the goroutines used to prime initial pool
65+
// connections in parallel. Pools smaller than this naturally fan out to
66+
// connPoolSize workers; larger pools cap here so we don't spawn one
67+
// dial+Prime goroutine per connection.
68+
maxPrimeWorkers = 10
6469
)
6570

6671
// ipProtocol represents the type of IP protocol used.
@@ -502,45 +507,23 @@ func NewBigtableChannelPool(ctx context.Context, connPoolSize int, strategy btop
502507
pool.selectFunc = pool.selectRoundRobin
503508
}
504509

505-
var exitSignal error
506510
btopt.Debugf(pool.logger, "bigtable_connpool: Creating conn pool with %d connections", connPoolSize)
507511
// TODO: Replace this logic with addConnections(...).
508512
initialConns := make([]*connEntry, connPoolSize)
509-
for i := 0; i < connPoolSize; i++ {
510-
select {
511-
case <-pool.poolCtx.Done():
512-
exitSignal = errors.New("bigtable_connpool: pool context canceled")
513-
default:
514-
}
515-
516-
if exitSignal != nil {
517-
break
518-
}
519-
520-
var entry *connEntry
521-
var err error
522-
523-
if i == 0 && firstConn != nil {
524-
entry = &connEntry{conn: firstConn}
525-
} else {
526-
entry, err = pool.factory.newEntry(ctx)
527-
}
528-
529-
if err != nil {
530-
exitSignal = err
531-
break
532-
}
533-
initialConns[i] = entry
513+
primeStart := 0
514+
if firstConn != nil {
515+
initialConns[0] = &connEntry{conn: firstConn}
516+
primeStart = 1
534517
}
535-
if exitSignal != nil {
536-
btopt.Debugf(pool.logger, "bigtable_connpool: error during initial connection creation: %v\n", exitSignal)
537-
// Close populated conns
518+
519+
if err := pool.primeInitialConns(pool.poolCtx, initialConns, primeStart); err != nil {
520+
btopt.Debugf(pool.logger, "bigtable_connpool: error during initial connection creation: %v\n", err)
538521
for _, entry := range initialConns {
539522
if entry != nil && entry.conn != nil {
540523
entry.conn.Close()
541524
}
542525
}
543-
return nil, exitSignal
526+
return nil, err
544527
}
545528

546529
pool.conns.Store(&initialConns)
@@ -569,6 +552,43 @@ func NewBigtableChannelPool(ctx context.Context, connPoolSize int, strategy btop
569552
return pool, nil
570553
}
571554

555+
// primeInitialConns dials and primes the connections at indices [primeStart, len(out))
556+
// in parallel, capped at maxPrimeWorkers. Successful entries are written into out at
557+
// their target index; if any prime fails, the first error is returned and the caller is
558+
// responsible for closing any populated entries.
559+
//
560+
// ctx scopes the prime operations: the first worker error (or ctx cancellation) tears
561+
// down the rest via errgroup's derived context.
562+
func (p *BigtableChannelPool) primeInitialConns(ctx context.Context, out []*connEntry, primeStart int) error {
563+
jobs := len(out) - primeStart
564+
if jobs <= 0 {
565+
return nil
566+
}
567+
if err := ctx.Err(); err != nil {
568+
return fmt.Errorf("bigtable_connpool: pool context canceled: %w", err)
569+
}
570+
571+
workers := jobs
572+
if workers > maxPrimeWorkers {
573+
workers = maxPrimeWorkers
574+
}
575+
576+
g, gctx := errgroup.WithContext(ctx)
577+
g.SetLimit(workers)
578+
for i := primeStart; i < len(out); i++ {
579+
idx := i
580+
g.Go(func() error {
581+
entry, err := p.factory.newEntry(gctx)
582+
if err != nil {
583+
return err
584+
}
585+
out[idx] = entry
586+
return nil
587+
})
588+
}
589+
return g.Wait()
590+
}
591+
572592
// checkIfDirectAccessCompatible attempts to create a single connection using the directAccessDialer,
573593
// primes it, and checks if direct access was successful
574594
func (p *BigtableChannelPool) checkIfDirectAccessCompatible() (*BigtableConn, bool) {

bigtable/internal/transport/connpool_helper_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type fakeService struct {
3636
btpb.UnimplementedBigtableServer
3737
mu sync.Mutex
3838
pingCount int
39+
pingInflight int // Currently in-flight PingAndWarm RPCs
40+
pingPeakInflight int // High-water mark for concurrent PingAndWarm RPCs
3941
callCount int
4042
streamSema chan struct{} // To control stream lifetime
4143
delay time.Duration // To simulate work
@@ -122,6 +124,8 @@ func (f *fakeService) reset() {
122124
defer f.mu.Unlock()
123125
f.callCount = 0
124126
f.pingCount = 0
127+
f.pingInflight = 0
128+
f.pingPeakInflight = 0
125129
f.serverErr = nil
126130
f.pingErrs = nil
127131
f.delay = 0
@@ -139,6 +143,10 @@ func (s *fakeService) PingAndWarm(ctx context.Context, req *btpb.PingAndWarmRequ
139143
s.mu.Lock()
140144
callNum := s.pingCount
141145
s.pingCount++
146+
s.pingInflight++
147+
if s.pingInflight > s.pingPeakInflight {
148+
s.pingPeakInflight = s.pingInflight
149+
}
142150

143151
var err error
144152
if len(s.pingErrs) > 0 {
@@ -156,6 +164,13 @@ func (s *fakeService) PingAndWarm(ctx context.Context, req *btpb.PingAndWarmRequ
156164
s.lastPingAndWarmMetadata, _ = metadata.FromIncomingContext(ctx)
157165
}
158166
s.mu.Unlock()
167+
168+
defer func() {
169+
s.mu.Lock()
170+
s.pingInflight--
171+
s.mu.Unlock()
172+
}()
173+
159174
if delay > 0 {
160175
select {
161176
case <-time.After(delay):
@@ -170,6 +185,12 @@ func (s *fakeService) PingAndWarm(ctx context.Context, req *btpb.PingAndWarmRequ
170185
return &btpb.PingAndWarmResponse{}, nil
171186
}
172187

188+
func (s *fakeService) getPingPeakInflight() int {
189+
s.mu.Lock()
190+
defer s.mu.Unlock()
191+
return s.pingPeakInflight
192+
}
193+
173194
func (s *fakeService) getCallCount() int {
174195
s.mu.Lock()
175196
defer s.mu.Unlock()

bigtable/internal/transport/connpool_test.go

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,10 +671,9 @@ func TestNewBigtableChannelPool(t *testing.T) {
671671

672672
t.Run("DialFailure", func(t *testing.T) {
673673
poolSize := 3
674-
dialCount := 0
674+
var dialCount atomic.Int64
675675
dialFunc := func() (*BigtableConn, error) {
676-
dialCount++
677-
if dialCount > 1 {
676+
if dialCount.Add(1) > 1 {
678677
return nil, errors.New("simulated dial error")
679678
}
680679
fake := &fakeService{}
@@ -689,6 +688,59 @@ func TestNewBigtableChannelPool(t *testing.T) {
689688
})
690689
}
691690

691+
// TestNewBigtableChannelPoolParallelPriming verifies that initial pool priming
692+
// fans out across multiple workers, capped at maxPrimeWorkers. The fake server
693+
// holds each PingAndWarm open for primeDelay so we can read the peak concurrent
694+
// in-flight count; with sequential priming the peak would be 1.
695+
func TestNewBigtableChannelPoolParallelPriming(t *testing.T) {
696+
ctx := context.Background()
697+
primeDelay := 200 * time.Millisecond
698+
699+
cases := []struct {
700+
name string
701+
poolSize int
702+
wantPeak int
703+
}{
704+
{name: "BelowWorkerCap", poolSize: 4, wantPeak: 4},
705+
{name: "AtWorkerCap", poolSize: maxPrimeWorkers, wantPeak: maxPrimeWorkers},
706+
{name: "AboveWorkerCap", poolSize: maxPrimeWorkers * 2, wantPeak: maxPrimeWorkers},
707+
}
708+
for _, tc := range cases {
709+
t.Run(tc.name, func(t *testing.T) {
710+
fake := &fakeService{}
711+
fake.setDelay(primeDelay)
712+
addr := setupTestServer(t, fake)
713+
dialFunc := func() (*BigtableConn, error) { return dialBigtableserver(addr) }
714+
715+
start := time.Now()
716+
pool, err := NewBigtableChannelPool(ctx, tc.poolSize, btopt.RoundRobin, dialFunc, time.Now(), poolOpts()...)
717+
elapsed := time.Since(start)
718+
if err != nil {
719+
t.Fatalf("NewBigtableChannelPool failed: %v", err)
720+
}
721+
defer pool.Close()
722+
723+
if got := fake.getPingPeakInflight(); got != tc.wantPeak {
724+
t.Errorf("peak in-flight PingAndWarm = %d, want %d", got, tc.wantPeak)
725+
}
726+
727+
// Sanity bound on wall time: at most ceil(poolSize / workers) prime
728+
// batches plus generous slack for dial/handshake overhead. A
729+
// sequential implementation would take ~poolSize * primeDelay.
730+
workers := tc.wantPeak
731+
batches := (tc.poolSize + workers - 1) / workers
732+
wantUnderSequential := time.Duration(tc.poolSize) * primeDelay
733+
wantAtMost := time.Duration(batches)*primeDelay + 2*time.Second
734+
if elapsed >= wantUnderSequential {
735+
t.Errorf("elapsed %v >= sequential bound %v — priming did not parallelize", elapsed, wantUnderSequential)
736+
}
737+
if elapsed > wantAtMost {
738+
t.Errorf("elapsed %v > expected upper bound %v", elapsed, wantAtMost)
739+
}
740+
})
741+
}
742+
}
743+
692744
func TestConnEntryCalculateConnLoadWithPenalty(t *testing.T) {
693745
entry := &connEntry{}
694746

0 commit comments

Comments
 (0)