Skip to content

Commit b32fbd7

Browse files
sushanbsushanb
authored
feat(bigtable): wire Diverter on Client and route Open* via TableShim (#20256)
## Summary Three coordinated changes so a follow-up session-data-path patch can flip traffic through the shim without further Client / OpenTable churn: 1. **Client owns a `*btransport.Diverter`**, constructed with `sessionLoad = 0.0` in `NewClientWithConfig`. Every call stays on the classic path until a future change wires the session backend and bumps the ratio. 2. **`Open` / `OpenTable` / `OpenAuthorizedView` / `OpenMaterializedView` move out of `client.go` into a new `open.go`.** `OpenTable` and the view variants now return `NewTableShim(classic, nil, c.diverter)` — same classic `tableImpl` inside, routing bolted on. `Open()` (returning `*Table`) stays classic-only for callers that need the concrete pointer type (BulkMutation etc.). 3. **`TableShim.pickSession()` gate.** The session `TableAPI` is optional; with `session == nil` we short-circuit **before** consulting the diverter — otherwise the pick-count histogram would record session picks that got silently downgraded here. `ReadRow` + `Apply` route through `pickSession()` instead of calling `diverter.UseSession()` directly. ## Also included: `MinConns` 10 → 4 `DefaultDynamicChannelPoolConfig.MinConns` drops from 10 to 4. The session client uses a pool size of 4 as its footprint default (small enough for the server-driven `GetClientConfiguration` reshape to take over quickly); with today's floor of 10 that trips `ValidateDynamicConfig` with: > `initial connPoolSize (4) must be between DynamicChannelPoolConfig.MinConns (10) and MaxConns (200)` Lowering the floor lets the session pool validate under the same default config the classic pool uses. No effect on the classic path: classic clients that don't call `WithGRPCConnectionPool` land at `defaultBigtableConnPoolSize=4` anyway, so the floor was already tighter than the actual default in practice. ## Behavior - Classic path unchanged. `sessionLoad = 0.0` makes `UseSession()` return `false` in every case (its `load <= 0` branch is short-circuit), so every `ReadRow` / `Apply` on a shimmed table lands on `t.classic`. - The `pickSession()` nil-guard is defense-in-depth for the case where the session backend isn't wired yet. - `Diverter.sessionPicks` / `classicPicks` counters start incrementing correctly the moment the session backend gets wired in — no debug UI needs to move. ## Files | File | Delta | Purpose | |---|---|---| | `bigtable/client.go` | -54 / -0 net after move | Add `diverter` field; strip `Open*` methods (moved) | | `bigtable/open.go` | +81 new | `Open` / `OpenTable` / `OpenAuthorizedView` / `OpenMaterializedView` | | `bigtable/table_shim.go` | +26 / -6 | `pickSession()` nil-safe gate | | `bigtable/internal/option/option.go` | +1 / -1 | `MinConns` 10 → 4 | Total: **+108 / -53** across 4 files. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./...` clean - [x] `go test ./bigtable/... -count=1 -short -timeout=180s` — all packages green including `bigtable` (13s), `internal/transport` (25s), `internal/option` (0s), `internal/session` (0s).
1 parent 35e146e commit b32fbd7

4 files changed

Lines changed: 274 additions & 50 deletions

File tree

‎bigtable/client.go‎

Lines changed: 6 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ type Client struct {
5252
executeQueryRetryOption gax.CallOption
5353
featureFlagsMD metadata.MD // Pre-computed feature flags metadata to be sent with each request.
5454
mPool btransport.ManagedChannelPool
55+
// diverter picks between the classic and (future) session data path
56+
// on every Open* return. Initialized with sessionLoad=0.0 so all
57+
// traffic stays on the classic path until a follow-up change enables
58+
// the session backend and bumps the ratio.
59+
diverter *btransport.Diverter
5560
}
5661

5762
// ClientConfig has configurations for the client.
@@ -227,6 +232,7 @@ func NewClientWithConfig(ctx context.Context, project, instance string, config C
227232
executeQueryRetryOption: executeQueryRetryOption,
228233
featureFlagsMD: directAccessMD,
229234
mPool: mPool,
235+
diverter: btransport.NewDiverter(0.0),
230236
}, nil
231237
}
232238

@@ -262,55 +268,6 @@ func (c *Client) reqParamsHeaderValInstance() string {
262268
return fmt.Sprintf("name=%s&app_profile_id=%s", url.QueryEscape(c.fullInstanceName()), url.QueryEscape(c.appProfile))
263269
}
264270

265-
// Open opens a table.
266-
func (c *Client) Open(table string) *Table {
267-
return &Table{
268-
c: c,
269-
table: table,
270-
md: metadata.Join(metadata.Pairs(
271-
resourcePrefixHeader, c.fullTableName(table),
272-
requestParamsHeader, c.reqParamsHeaderValTable(table),
273-
), c.featureFlagsMD),
274-
}
275-
}
276-
277-
// OpenTable opens a table.
278-
func (c *Client) OpenTable(table string) TableAPI {
279-
return &tableImpl{Table{
280-
c: c,
281-
table: table,
282-
md: metadata.Join(metadata.Pairs(
283-
resourcePrefixHeader, c.fullTableName(table),
284-
requestParamsHeader, c.reqParamsHeaderValTable(table),
285-
), c.featureFlagsMD),
286-
}}
287-
}
288-
289-
// OpenAuthorizedView opens an authorized view.
290-
func (c *Client) OpenAuthorizedView(table, authorizedView string) TableAPI {
291-
return &tableImpl{Table{
292-
c: c,
293-
table: table,
294-
md: metadata.Join(metadata.Pairs(
295-
resourcePrefixHeader, c.fullAuthorizedViewName(table, authorizedView),
296-
requestParamsHeader, c.reqParamsHeaderValTable(table),
297-
), c.featureFlagsMD),
298-
authorizedView: authorizedView,
299-
}}
300-
}
301-
302-
// OpenMaterializedView opens a materialized view.
303-
func (c *Client) OpenMaterializedView(materializedView string) TableAPI {
304-
return &tableImpl{Table{
305-
c: c,
306-
md: metadata.Join(metadata.Pairs(
307-
resourcePrefixHeader, c.fullMaterializedViewName(materializedView),
308-
requestParamsHeader, c.reqParamsHeaderValTable(materializedView),
309-
), c.featureFlagsMD),
310-
materializedView: materializedView,
311-
}}
312-
}
313-
314271
// PingAndWarm pings the server and warms up the connection.
315272
func (c *Client) PingAndWarm(ctx context.Context) (err error) {
316273
md := metadata.Join(metadata.Pairs(

‎bigtable/internal/option/option.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ type DynamicChannelPoolConfig struct {
260260
func DefaultDynamicChannelPoolConfig() DynamicChannelPoolConfig {
261261
return DynamicChannelPoolConfig{
262262
Enabled: false,
263-
MinConns: 10,
263+
MinConns: 4,
264264
MaxConns: 200,
265265
AvgLoadHighThreshold: 50,
266266
AvgLoadLowThreshold: 5,

‎bigtable/open.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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 bigtable
16+
17+
import "google.golang.org/grpc/metadata"
18+
19+
// Open opens a table for use with the classic data path helpers that
20+
// still take *Table directly (BulkMutation, etc.). The returned *Table
21+
// is always the classic implementation regardless of the Client's
22+
// session-load ratio — callers who want the divertible surface should
23+
// use OpenTable / OpenAuthorizedView / OpenMaterializedView.
24+
func (c *Client) Open(table string) *Table {
25+
return &Table{
26+
c: c,
27+
table: table,
28+
md: metadata.Join(metadata.Pairs(
29+
resourcePrefixHeader, c.fullTableName(table),
30+
requestParamsHeader, c.reqParamsHeaderValTable(table),
31+
), c.featureFlagsMD),
32+
}
33+
}
34+
35+
// OpenTable opens a table. Returns a TableShim that routes each RPC via
36+
// the Client's Diverter — with sessionLoad=0.0 (the default at
37+
// construction time) every call lands on the classic path. When a
38+
// follow-up change wires in the session data path, callers get session
39+
// routing automatically without re-opening the table.
40+
func (c *Client) OpenTable(table string) TableAPI {
41+
classic := &tableImpl{Table{
42+
c: c,
43+
table: table,
44+
md: metadata.Join(metadata.Pairs(
45+
resourcePrefixHeader, c.fullTableName(table),
46+
requestParamsHeader, c.reqParamsHeaderValTable(table),
47+
), c.featureFlagsMD),
48+
}}
49+
return NewTableShim(classic, nil, c.diverter)
50+
}
51+
52+
// OpenAuthorizedView opens an authorized view. See OpenTable for the
53+
// diverter routing story.
54+
func (c *Client) OpenAuthorizedView(table, authorizedView string) TableAPI {
55+
classic := &tableImpl{Table{
56+
c: c,
57+
table: table,
58+
md: metadata.Join(metadata.Pairs(
59+
resourcePrefixHeader, c.fullAuthorizedViewName(table, authorizedView),
60+
requestParamsHeader, c.reqParamsHeaderValTable(table),
61+
), c.featureFlagsMD),
62+
authorizedView: authorizedView,
63+
}}
64+
return NewTableShim(classic, nil, c.diverter)
65+
}
66+
67+
// OpenMaterializedView opens a materialized view. See OpenTable for the
68+
// diverter routing story.
69+
func (c *Client) OpenMaterializedView(materializedView string) TableAPI {
70+
classic := &tableImpl{Table{
71+
c: c,
72+
md: metadata.Join(metadata.Pairs(
73+
resourcePrefixHeader, c.fullMaterializedViewName(materializedView),
74+
requestParamsHeader, c.reqParamsHeaderValTable(materializedView),
75+
), c.featureFlagsMD),
76+
materializedView: materializedView,
77+
}}
78+
return NewTableShim(classic, nil, c.diverter)
79+
}

‎bigtable/open_test.go‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
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 bigtable
16+
17+
import (
18+
"testing"
19+
20+
btransport "cloud.google.com/go/bigtable/internal/transport"
21+
"google.golang.org/grpc/metadata"
22+
)
23+
24+
// newBareClientForOpenTests builds a Client with just enough state for
25+
// the Open* factory paths — no gRPC connection, no metrics tracer, no
26+
// pools. Sufficient because Open* only reads project/instance/
27+
// appProfile/featureFlagsMD/diverter and constructs Table + TableShim
28+
// values without dialing.
29+
func newBareClientForOpenTests(t *testing.T, sessionLoad float64) *Client {
30+
t.Helper()
31+
return &Client{
32+
project: "p",
33+
instance: "i",
34+
appProfile: "ap",
35+
featureFlagsMD: metadata.MD{},
36+
diverter: btransport.NewDiverter(sessionLoad),
37+
}
38+
}
39+
40+
// TestOpen_ReturnsBareTable pins the post-Option-B-revert contract:
41+
// Client.Open returns a plain *Table with no session-routing wrapper.
42+
// Callers holding *Table (BulkMutation, ReadModifyWrite consumers,
43+
// external code) stay on the classic path regardless of Client's
44+
// diverter setting.
45+
func TestOpen_ReturnsBareTable(t *testing.T) {
46+
c := newBareClientForOpenTests(t, 0.0)
47+
48+
tbl := c.Open("mytable")
49+
if tbl == nil {
50+
t.Fatal("Open returned nil")
51+
}
52+
if tbl.c != c {
53+
t.Errorf("Open→Table.c = %p, want %p", tbl.c, c)
54+
}
55+
if tbl.table != "mytable" {
56+
t.Errorf("Open→Table.table = %q, want %q", tbl.table, "mytable")
57+
}
58+
if tbl.authorizedView != "" {
59+
t.Errorf("Open→Table.authorizedView = %q, want empty", tbl.authorizedView)
60+
}
61+
if tbl.materializedView != "" {
62+
t.Errorf("Open→Table.materializedView = %q, want empty", tbl.materializedView)
63+
}
64+
}
65+
66+
// TestOpenTable_ProducesNilSessionShim pins the shim shape returned by
67+
// OpenTable: a *TableShim whose classic side is a *tableImpl and whose
68+
// session side is nil (session data path isn't wired here). The
69+
// client's Diverter is passed through so a future ratio bump takes
70+
// effect without re-opening. The already-existing
71+
// TestTableShim_NilSession_AllMethodsFallBackToClassic covers the
72+
// behavioral consequence — with session == nil, every routing decision
73+
// falls through to classic.
74+
func TestOpenTable_ProducesNilSessionShim(t *testing.T) {
75+
c := newBareClientForOpenTests(t, 1.0) // SessionLoad=1.0 to prove the shim still picks classic when session is nil.
76+
77+
got := c.OpenTable("mytable")
78+
shim, ok := got.(*TableShim)
79+
if !ok {
80+
t.Fatalf("OpenTable returned %T, want *TableShim", got)
81+
}
82+
if shim.session != nil {
83+
t.Errorf("OpenTable→TableShim.session = %v, want nil (session data path not wired)", shim.session)
84+
}
85+
if shim.diverter != c.diverter {
86+
t.Errorf("OpenTable→TableShim.diverter = %p, want client's diverter %p", shim.diverter, c.diverter)
87+
}
88+
inner, ok := shim.classic.(*tableImpl)
89+
if !ok {
90+
t.Fatalf("OpenTable→TableShim.classic = %T, want *tableImpl", shim.classic)
91+
}
92+
if inner.table != "mytable" {
93+
t.Errorf("classic inner Table.table = %q, want %q", inner.table, "mytable")
94+
}
95+
if inner.authorizedView != "" {
96+
t.Errorf("classic inner Table.authorizedView = %q, want empty", inner.authorizedView)
97+
}
98+
if inner.materializedView != "" {
99+
t.Errorf("classic inner Table.materializedView = %q, want empty", inner.materializedView)
100+
}
101+
// useSession() must be false because session is nil, even with
102+
// SessionLoad=1.0 on the diverter — proves the nil-session
103+
// short-circuit runs before the diverter is consulted.
104+
if shim.useSession() {
105+
t.Errorf("useSession() = true, want false (session is nil so we must not consult the diverter)")
106+
}
107+
}
108+
109+
// TestOpenAuthorizedView_ProducesNilSessionShim — same contract as
110+
// OpenTable, plus authorizedView is threaded through to the inner
111+
// Table.
112+
func TestOpenAuthorizedView_ProducesNilSessionShim(t *testing.T) {
113+
c := newBareClientForOpenTests(t, 0.0)
114+
115+
got := c.OpenAuthorizedView("mytable", "myview")
116+
shim, ok := got.(*TableShim)
117+
if !ok {
118+
t.Fatalf("OpenAuthorizedView returned %T, want *TableShim", got)
119+
}
120+
if shim.session != nil {
121+
t.Errorf("session = %v, want nil", shim.session)
122+
}
123+
if shim.diverter != c.diverter {
124+
t.Errorf("diverter = %p, want client's diverter %p", shim.diverter, c.diverter)
125+
}
126+
inner, ok := shim.classic.(*tableImpl)
127+
if !ok {
128+
t.Fatalf("classic = %T, want *tableImpl", shim.classic)
129+
}
130+
if inner.table != "mytable" {
131+
t.Errorf("classic table = %q, want %q", inner.table, "mytable")
132+
}
133+
if inner.authorizedView != "myview" {
134+
t.Errorf("classic authorizedView = %q, want %q", inner.authorizedView, "myview")
135+
}
136+
}
137+
138+
// TestOpenMaterializedView_ProducesNilSessionShim — same contract as
139+
// OpenTable, plus materializedView is threaded through to the inner
140+
// Table (and table is empty since MVs are addressed by view name only).
141+
func TestOpenMaterializedView_ProducesNilSessionShim(t *testing.T) {
142+
c := newBareClientForOpenTests(t, 0.0)
143+
144+
got := c.OpenMaterializedView("myview")
145+
shim, ok := got.(*TableShim)
146+
if !ok {
147+
t.Fatalf("OpenMaterializedView returned %T, want *TableShim", got)
148+
}
149+
if shim.session != nil {
150+
t.Errorf("session = %v, want nil", shim.session)
151+
}
152+
if shim.diverter != c.diverter {
153+
t.Errorf("diverter = %p, want client's diverter %p", shim.diverter, c.diverter)
154+
}
155+
inner, ok := shim.classic.(*tableImpl)
156+
if !ok {
157+
t.Fatalf("classic = %T, want *tableImpl", shim.classic)
158+
}
159+
if inner.materializedView != "myview" {
160+
t.Errorf("classic materializedView = %q, want %q", inner.materializedView, "myview")
161+
}
162+
if inner.table != "" {
163+
t.Errorf("classic table = %q, want empty (MV addressed by view name only)", inner.table)
164+
}
165+
}
166+
167+
// TestOpenFactories_ShareOneClientDiverter pins that every Open*
168+
// factory returns a shim referencing the SAME *Diverter — so a future
169+
// SetSessionLoad call from a ConfigurationManager updates every open
170+
// resource on the client at once, no per-resource iteration.
171+
func TestOpenFactories_ShareOneClientDiverter(t *testing.T) {
172+
c := newBareClientForOpenTests(t, 0.0)
173+
174+
tbl := c.OpenTable("t").(*TableShim)
175+
av := c.OpenAuthorizedView("t", "v").(*TableShim)
176+
mv := c.OpenMaterializedView("mv").(*TableShim)
177+
178+
if tbl.diverter != c.diverter || av.diverter != c.diverter || mv.diverter != c.diverter {
179+
t.Errorf("diverters diverge: tbl=%p av=%p mv=%p client=%p",
180+
tbl.diverter, av.diverter, mv.diverter, c.diverter)
181+
}
182+
// Flip the client's diverter to prove the reference is live, not
183+
// a copy.
184+
c.diverter.SetSessionLoad(0.42)
185+
if got := tbl.diverter.SessionLoad(); got != 0.42 {
186+
t.Errorf("after SetSessionLoad(0.42), tbl.diverter.SessionLoad() = %v, want 0.42", got)
187+
}
188+
}

0 commit comments

Comments
 (0)