close
Skip to content

Commit f6ae3fb

Browse files
authored
feat(bigtable): add lazyPool helper for on-demand session pool opening (#20182)
## Summary - Adds `bigtable/internal/session` with a `lazyPool` primitive that opens its underlying `Invoker` on first use. Concurrent callers block until the open completes; failed opens are NOT cached, so a transient `proto.Marshal` failure cannot strand the caller for the process lifetime. - A nil `*lazyPool` or one with a nil `open` closure returns `(nil, nil)`, letting callers model "no session support, use fallback" (e.g., the write side of a read-only materialized view). - Adds `transport.InvokeResult` — the value type returned by `Session.Invoke` — so the session package can declare the `Invoker` interface without pulling in the full session-pool implementation, which will land in a follow-up. ## Test plan - [x] `go build ./bigtable/internal/session/... ./bigtable/internal/transport/...` - [x] `go vet ./bigtable/internal/session/... ./bigtable/internal/transport/...` - [x] `go test ./bigtable/internal/session/... -run LazyPool -count=1` (both `TestLazyPool_NilPoolAndNilOpenReturnNilNil` and `TestLazyPool_FailedOpenNotCached` pass) - [x] `go test ./bigtable/internal/session/... -count=1 -race`
1 parent 4b82fd2 commit f6ae3fb

3 files changed

Lines changed: 256 additions & 0 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package session
16+
17+
import (
18+
"context"
19+
"sync"
20+
21+
btransport "cloud.google.com/go/bigtable/internal/transport"
22+
)
23+
24+
// Invoker is the narrow surface sessionTable needs from a session
25+
// pool: dispatch a single virtual RPC and surface the full
26+
// InvokeResult (response, cluster info, server-side Stats, and the
27+
// local SentAt timestamp). Satisfied by *btransport.SessionPoolImpl;
28+
// the interface exists so tests can substitute a fake without
29+
// standing up a real pool.
30+
type Invoker interface {
31+
Invoke(ctx context.Context, desc btransport.VRpcDescriptor, req interface{}) (btransport.InvokeResult, error)
32+
}
33+
34+
// lazyPool wraps an Invoker (typically *btransport.SessionPoolImpl)
35+
// that is opened on first use. Callers invoke get(); the first
36+
// winner runs the open closure (synchronously — dial + handshake
37+
// happens here) and stores the result; subsequent callers see the
38+
// stored pool with no work.
39+
//
40+
// Failed opens are NOT cached: the next caller retries. This
41+
// matters because pool creation can fail transiently (proto.Marshal
42+
// is the only obvious source today, but future descriptor variants
43+
// may add more). A permanent error-cache would leave the sessionTable
44+
// stuck for the process lifetime.
45+
//
46+
// A nil *lazyPool or one with a nil open closure returns (nil, nil)
47+
// — "no session support, use fallback." Used for the write side of
48+
// materialized views (read-only).
49+
type lazyPool struct {
50+
mu sync.Mutex
51+
pool Invoker
52+
open func() (Invoker, error)
53+
}
54+
55+
// get returns the underlying pool, opening it on first call.
56+
// Concurrent callers block until the open completes.
57+
func (l *lazyPool) get() (Invoker, error) {
58+
if l == nil || l.open == nil {
59+
return nil, nil
60+
}
61+
l.mu.Lock()
62+
defer l.mu.Unlock()
63+
if l.pool != nil {
64+
return l.pool, nil
65+
}
66+
p, err := l.open()
67+
if err != nil {
68+
return nil, err
69+
}
70+
l.pool = p
71+
return p, nil
72+
}
73+
74+
// opened reports whether the pool has been opened yet — for tests
75+
// and for the sessionz debug UI which wants to render "read pool:
76+
// not yet opened" vs a live pool.
77+
func (l *lazyPool) opened() bool {
78+
if l == nil {
79+
return false
80+
}
81+
l.mu.Lock()
82+
defer l.mu.Unlock()
83+
return l.pool != nil
84+
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package session
16+
17+
import (
18+
"context"
19+
"errors"
20+
"sync"
21+
"testing"
22+
23+
btransport "cloud.google.com/go/bigtable/internal/transport"
24+
)
25+
26+
// stubInvoker is a minimal Invoker for tests that only need identity —
27+
// distinct from the shared fakeInvoker to avoid coupling this test file
28+
// to table_test.go's fixture.
29+
type stubInvoker struct{ tag string }
30+
31+
func (s *stubInvoker) Invoke(_ context.Context, _ btransport.VRpcDescriptor, _ interface{}) (btransport.InvokeResult, error) {
32+
return btransport.InvokeResult{}, nil
33+
}
34+
35+
// TestLazyPool_NilPoolAndNilOpenReturnNilNil verifies the "no session
36+
// support" contract: a nil *lazyPool or one with a nil open closure
37+
// returns (nil, nil). Used for the write side of MatView (spec #11).
38+
func TestLazyPool_NilPoolAndNilOpenReturnNilNil(t *testing.T) {
39+
var nilPool *lazyPool
40+
if p, err := nilPool.get(); p != nil || err != nil {
41+
t.Errorf("nil-receiver get() = (%v, %v), want (nil, nil)", p, err)
42+
}
43+
44+
empty := &lazyPool{}
45+
if p, err := empty.get(); p != nil || err != nil {
46+
t.Errorf("nil-open get() = (%v, %v), want (nil, nil)", p, err)
47+
}
48+
if empty.opened() {
49+
t.Error("empty.opened() = true, want false")
50+
}
51+
}
52+
53+
// TestLazyPool_FailedOpenNotCached verifies SESSION_SPEC.md #11: failed
54+
// opens MUST NOT be cached. A transient proto.Marshal failure (or any
55+
// other opener error) MUST NOT strand the table for the process
56+
// lifetime. The next get() call re-invokes open().
57+
//
58+
// The counterfactual: if we DID cache the failure, calls == 1 after N
59+
// gets. The invariant is that calls == N.
60+
func TestLazyPool_FailedOpenNotCached(t *testing.T) {
61+
var (
62+
mu sync.Mutex
63+
calls int
64+
failNext = true
65+
succeed = &stubInvoker{tag: "opened"}
66+
wantErr = errors.New("marshal boom")
67+
)
68+
l := &lazyPool{
69+
open: func() (Invoker, error) {
70+
mu.Lock()
71+
defer mu.Unlock()
72+
calls++
73+
if failNext {
74+
return nil, wantErr
75+
}
76+
return succeed, nil
77+
},
78+
}
79+
80+
// Four failing opens — each MUST re-invoke the closure.
81+
for i := 0; i < 4; i++ {
82+
p, err := l.get()
83+
if p != nil {
84+
t.Errorf("call #%d: pool = %v, want nil on open failure", i+1, p)
85+
}
86+
if !errors.Is(err, wantErr) {
87+
t.Errorf("call #%d: err = %v, want %v", i+1, err, wantErr)
88+
}
89+
if l.opened() {
90+
t.Errorf("call #%d: opened() = true after failure; failure MUST NOT be cached", i+1)
91+
}
92+
}
93+
mu.Lock()
94+
if calls != 4 {
95+
t.Errorf("open() invocations after 4 failing gets = %d, want 4 — failed opens were cached, violating spec #11", calls)
96+
}
97+
// Now flip: next open succeeds. Subsequent gets MUST return the
98+
// cached success (open() invoked exactly one more time, not four).
99+
failNext = false
100+
mu.Unlock()
101+
102+
for i := 0; i < 4; i++ {
103+
p, err := l.get()
104+
if err != nil {
105+
t.Errorf("post-success call #%d: err = %v, want nil", i+1, err)
106+
}
107+
if p != succeed {
108+
t.Errorf("post-success call #%d: pool = %v, want cached stubInvoker", i+1, p)
109+
}
110+
}
111+
mu.Lock()
112+
defer mu.Unlock()
113+
if calls != 5 {
114+
t.Errorf("total open() invocations = %d, want 5 (4 failing + 1 successful cached) — successful opens MUST be cached", calls)
115+
}
116+
if !l.opened() {
117+
t.Error("opened() = false after successful open, want true")
118+
}
119+
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package internal
16+
17+
import (
18+
"time"
19+
20+
spb "cloud.google.com/go/bigtable/apiv2/bigtablepb"
21+
)
22+
23+
// InvokeResult carries the full set of outputs from a single Invoke call.
24+
//
25+
// Fields:
26+
// - Response: decoded vRPC payload (typed per VRpcDescriptor.Decode); nil on error.
27+
// - ClusterInfo: server-reported routing/cluster identity; may be set on
28+
// both success and error paths if the server included it.
29+
// - Stats: server-reported per-request statistics (notably BackendLatency);
30+
// nil if the server did not populate Stats on the success frame.
31+
// - SentAt: local monotonic timestamp captured immediately before the vRPC
32+
// frame was handed to the bidi Send. Used downstream to derive
33+
// client-side blocking latency (sentAt - attemptStart).
34+
//
35+
// ErrorResponse.RetryInfo from the server is plumbed via the returned error
36+
// using gRPC status details — callers can extract it with
37+
// status.FromError(err).Details() and type-asserting to *errdetails.RetryInfo.
38+
type InvokeResult struct {
39+
Response interface{}
40+
ClusterInfo *spb.ClusterInformation
41+
Stats *spb.SessionRequestStats
42+
// SentAt is a local monotonic timestamp captured immediately before
43+
// the vRPC frame is handed to the bidi Send. Used downstream to
44+
// derive client-side blocking latency (sentAt - attemptStart).
45+
SentAt time.Time
46+
PeerInfo *spb.PeerInfo
47+
// RPCIDOnSession is the per-session monotonic id of this call
48+
// (1, 2, 3, …). Distinguishes warm-up vRPCs (small id) from
49+
// established-session vRPCs.
50+
RPCIDOnSession int64
51+
// TransportLatency = AttemptLatency - BackendLatency.
52+
TransportLatency time.Duration
53+
}

0 commit comments

Comments
 (0)