Sushan Bhattarai
90d · built 2026-08-09
90-day totals
- Commits
- 64
- Grow
- 24.4
- Maintenance
- 9.0
- Fixes
- 2.0
- Total ETV
- 35.5
30-day trajectory
Last 30 days vs. the 30 days before. Up arrows on Growth and ETV mean improvement; up arrow on Fixes share means more time on fixes (worse).
↑+500.0 %
vs 8 prior
↑+50.8 pp
recent vs prior
↑+6.4 pp
recent vs prior
Daily performance
Daily ETV, stacked by Growth, Maintenance and Fixes.
Work-mix over time
Share of Growth / Maintenance / Fixes over a rolling 7-day window. Reads as 'where is effort flowing right now'.
Repository spread
Where this developer's commits land. Concentrated work (top1 > 80%) vs polymath spread (top1 < 30%).
| Repo | Commits | ETV |
|---|---|---|
| google-cloud-go | 61 | 35.5 |
Most impactful commits
Top 20 by ETV in the 90-day window.
- 4.6ETVfeat(bigtable): add SessionPoolImpl (two-tier pool + scaling + debug) (#20225) ## Summary Second of five PRs porting the session pool infrastructure from `feat/bigtable-sessionz-debug` (which powers the recycle-repro fleet). Adds `SessionPoolImpl`: the concrete two-tier read/write session pool for one resource. ~4000 LOC across five files plus matching tests. ## Stack - [ ] PR-1: sessionList (#20224) — per-AFE bucketing data structure. **Not yet merged.** - [x] **PR-2 (this)** — SessionPoolImpl (pool + scaling + debug + snapshot). - [ ] PR-3 — `SessionPool` / `Invoker` interfaces + `sessionClient` / `sessionTable` factory wiring. - [ ] PR-4 — Debug pages (`sessionz` / `afez` / `flightz` / `loadz` under `bigtable/debugview/`). - [ ] PR-5 — `bigtable.Client` integration + release notes. **Because PR-1 has not landed, this PR is opened against `main` and the diff includes PR-1's commits.** Once #20224 merges the base can be re-targeted (or this branch rebased) so only PR-2's own delta shows. ## What lands **SessionPoolImpl** (5 files, ~2400 LOC prod + ~2200 LOC tests): - `session_pool.go` — struct + constructor + `Invoke` + `CheckoutSession` (waiter queue, deadline propagation) + pluggable picker via the AFE picker from #20204. - `session_pool_lifecycle.go` — `SessionHooks` wiring, consecutive-failure breaker, `Close` (5-phase teardown), `WaitGoroutines` / `spawns.Wait` choreography so no session-owned goroutine outlives the pool. - `session_pool_scaling.go` — `Tick` loop, `createSession` (dial + `OpenSession` + hook registration), `pendingStarts` / `startingSessions` accounting so scale-up decisions never double-count in-flight opens. Uses the channel-pool pick hint (`ChannelPickHintInto`, added to `connpool.go`) to attribute each session to its underlying channel. - `session_pool_debug.go` — `PoolSnapshot` / slow-vRPC ring / per-close-reason counters / scaling-history buffer / `pickHistory` ring — the input to the sessionz / afez / loadz debug pages (landing in a later PR). - `session_snapshot.go` — the value-typed snapshot record the debug surface consumes; no live locks escape. **Session helpers added** (pool-facing additions to files already touched by prior PRs, kept minimal): - `Session.loops sync.WaitGroup` + `WaitGoroutines()` — pool teardown blocks on this so `readLoop` / `heartbeatLoop` and their `notifyClosed → recordClose` callback chains fully unwind before `Close` returns. Prevents session goroutines from racing metric-var writes across test boundaries. - `Session.closeErr atomic.Pointer[error]` + `setCloseErr` / `closeError` — preserves the raw `Recv` error handed to `handleClose`. Pool surfaces this on consecutive-failure breaker trips so operators see the underlying server rejection (e.g. `FailedPrecondition` when the resource is still being created) instead of only the sentinel. **Supporting additions to existing files:** - `afe_picker.go` — const `defaultAfeRandomSubsetSize = 2` (power-of-two-choices K-choice default; matches Java). - `debug_tracer.go` — three new tag constants: `tagSessionPoolCreatePanic`, `tagSessionPoolConsecutiveFailuresTripped`, `tagSessionPoolCheckoutFailedCINil`. - `connpool.go` — `ChannelPickHintInto(ctx, *atomic.Int32)` context helper. No-op when the channel pool doesn't consume the hint. ## What does NOT land yet - `SessionPool` / `Invoker` interfaces (follow-up PR alongside sessionClient / sessionTable). - `bigtable.Client` integration (PR-3+). - Debug pages under `bigtable/debugview/` (later PR). ## Test plan - [x] `go build ./...` passes. - [x] `go vet ./internal/transport/` clean. - [x] `go test ./internal/transport/ -race -count=1 -short -skip 'AfeLbSim' -timeout=180s` — passes (32s wall). ~2200 LOC of new tests across pool lifecycle, scaling, consecutive-failure breaker, AFE integration, debug surface, snapshot rendering, plus a K-choice bench. --- # Reviewer guide ## Guide 1 — mutianf (human) ### What this PR does Adds `SessionPoolImpl`, the layer that sits above the per-AFE `sessionList` shipped in #20224 and consumes it via a two-tier picker (AFE first, then a ready session in that AFE). It owns the session lifecycle (open / active / closing / close hooks), server-driven scaling via `PoolSizer`, a consecutive-failure circuit breaker, and the debug/observability surface (histograms + ring buffers) that feeds sessionz/loadz. New files: 5 source, 6 test, ~4.9k LOC. Nothing outside `session_pool*.go` / `session_snapshot*.go` is new logic — the small edits elsewhere are hook-plumbing scaffolding already vetted by the session/AFE subagent reviewers. ### Recommended read order 1. **`session_pool.go`** — start here. Struct field layout with per-field ownership comments (`:104-179`), the `waiter` FIFO shape (`:94-101`), `CheckoutSession` two-tier pick + parking (`:235-310`), `Invoke` (`:465-559`), `Stats` (`:361-397`), `UpdateConfig` (`:402-431`), `pickerFromLoadBalancing` (`:439-461`). Skim `session_pool_test.go` (28 tests) — the FIFO waiter, Stats, and UpdateConfig behaviors are all covered there. 2. **`session_pool_lifecycle.go`** — hooks (`onActive:255`, `onClosing:308`, `onClose:336`), `recordSessionClose` once-CAS on `Session.poolCloseRecorded` (`:117-130`), `Close`'s 6-phase teardown (`:154-247`), `noteAbnormalCloseIfAny` breaker (`:363-392`), the three ticker loops (`:426-538`). Skim `session_pool_lifecycle_test.go` — every hook + `Close`. 3. **`session_pool_scaling.go`** — `Tick` (`:81-162`), `createSession` worker (`:164-274`), `scalingReason` (`:278-299`), `noDeadlineButCancellableContext` (`:301-311`). Skim `session_pool_scaling_test.go` — the `scalingInProgress` gate and panic-safety are the only non-obvious contracts. 4. **`session_pool_debug.go`** — `poolMetrics` (`:36-72`), `latencyHist` log2 histogram (`:160-228`), the four ring buffers (slow-vRPC, time-series, lifetimes, pick-history), `recordPickDecision` (`:366-387`). Skim `session_pool_debug_test.go` — mostly ring-cap and rate-computation coverage. 5. **`session_snapshot.go`** — mostly type defs. Focus on `PoolSnapshot` (`:452-594`) and `LoadBalancingSnapshot` (`:414-436`) as the debug-view contract. 6. **`session_pool_consecutive_failures_test.go`** and **`session_pool_afe_test.go`** — end-to-end behavior verification; useful for confirming intent. ### Flow of events - **CheckoutSession → Invoke → release.** `CheckoutSession` (`session_pool.go:235`) opportunistically kicks Tick if `sl.ReadyCount()==0`, snapshots the picker under `p.mu`, then two-tier picks outside the lock: `ReadyAfes()` → `PickAfe` → `Checkout(afeID)` (`:259-268`). Miss → park in the FIFO waiter queue (`:286-289`), bracket `waitersCount` for the sizer (`:291,300`). `Invoke` (`:465`) checks out, runs `sh.session.Invoke`, records latencies (`:508-523`), logs a slow-vRPC row if over threshold (`:524-557`); the deferred `sh.DecOutstanding()` + `noteVRpcOutcome` (`:493-496`) hands the OK-gated latency to the per-AFE PeakEwma tracker. Session release itself is driven by `OnSlotDrained` (installed at `session_pool_scaling.go:228-231`), which returns the handle to `sessionList` and calls `signalFree` — separate from the `defer` in `Invoke`. - **Background Tick.** `startTickLoop` (`session_pool_lifecycle.go:426`) fires every 1 s → `tickOnce` debounces via `tickPending` CAS (`:447-458`) → `Tick` (`session_pool_scaling.go:81`) samples uptimes, gates on `scalingInProgress`, calls `sizer.Decide()`, and on a positive delta reserves `pendingStarts += delta` + `spawns.Add(delta)` under `p.mu` (`:131-138`) then fans out one goroutine per session. Each `createSession` acquires the budget outside `p.mu`, dials via `streamFactory`, transfers `pendingStarts → startingSessions` in one lock (`:246-249`), starts the session, and blocks on `WaitGoroutines` so it stays on `p.spawns` until the session dies. - **Abnormal close → breaker trip.** `onClose` (`session_pool_lifecycle.go:336`) CAS's `closeRecorded`, calls `noteAbnormalCloseIfAny` (`:363`), which bumps `consecutiveFailures` and stores the raw error into `lastAbnormalCloseErr`. Crossing the threshold snapshots the poison, CAS-resets the counter, and calls `drainWaitersWithErr` — waiters get `*consecutiveFailureError` wrapping the last cause (so `errors.Is(err, ErrConsecutiveFailures)` and `status.Code(err)` both still work, `:60-82`). Counter only resets in `onActive` (`:292-293`) — a successful open, not a healthy vRPC. ### Key invariants 1. **Two-tier pick, no re-entrant `p.mu`.** `CheckoutSession` reads `p.picker` under `p.mu` (`session_pool.go:249-255`) then unlocks before calling picker/sessionList. `recordPickDecision` takes `pickerName` as a **parameter** (`session_pool_debug.go:366`, `session_pool.go:260-262`) precisely because the caller already holds no lock — but any new pool method that reads `p.picker.Name()` from a hot path must not re-take `p.mu`. 2. **Waiter FIFO with `waitersCount` bracketed.** Every `PushBack` bumps `waitersCount` (`session_pool.go:291`); every wake path (`ctx.Done`, `w.ready`) decrements it (`:294,300`). `removeWaiter` (`:316`) is idempotent via `w.elem != nil`; `signalFree` and `drainWaitersWithErr` nil out `elem` under `waitersMu` (`:329-358`). `Stats().PendingCount` reads `waitersCount.Load()` — this is the sizer's queue-depth input. 3. **Close-exactly-once accounting.** `sessionsClosed` and `closesByReason` bumps are gated by `Session.poolCloseRecorded.CompareAndSwap(false, true)` inside `recordSessionClose` (`session_pool_lifecycle.go:117-130`). `sh.closingRecorded` and `sh.closeRecorded` are per-handle CAS's protecting the lifetime histogram + the `OnClose` branch. `Close`'s Phase 1 pre-flips both CAS's on every handle (`:187-193`) so a concurrent mid-flight onClosing can't double-count. 4. **Breaker resets only on `onActive`.** `consecutiveFailures.Store(0)` and `lastAbnormalCloseErr.Store(nil)` live at `session_pool_lifecycle.go:292-293`. Not on per-vRPC OK — otherwise one long-lived healthy session would mask a run of failed opens. 5. **Hot path is atomics/RLocks; debug views take snapshots.** `Stats` is the only per-request path that briefly takes `p.mu` (`session_pool.go:362`); everything else on the vRPC path is atomic. Debug snapshotters copy under lock and format after release (`session_snapshot.go:452-594`). ### What NOT to worry about - **Session / vRPC layer itself** — shipped in #20213 / #20215 (state machine, one-in-flight, PeerInfo timing, retry oracle, heartbeat). - **Per-AFE `sessionList` I1-I6** — shipped in #20224, has its own tests. - **`PoolSizer` scaling formula** — already upstream (`pool_sizer.go`); this PR only wires it and consumes `ScaleDecision`. - **AFE pickers (`SimpleAfePicker` / `LeastInFlight` / `LeastLatency`)** — already upstream (`afe_picker.go`); this PR only builds them via `pickerFromLoadBalancing`. - **`SessionThrottler` / `AdaptiveSessionThrottler`** — already upstream; this PR consumes `Acquire` / `Release` / `UpdateConfig`. - **`ClientConfigurationManager` polling** — this pool receives `UpdateConfig` calls; the polling itself is elsewhere. ### Danger zones - **Re-entrant `p.mu` on picker access.** `recordPickDecision` intentionally takes `pickerName` as a param (`session_pool_debug.go:366`). Adding a new pool method that reads `p.picker.Name()` from within a `CheckoutSession` code path is a re-entrant deadlock; pass the name in or snapshot up-front. - **`startingSessions` / `pendingStarts` accounting.** Tick reserves `pendingStarts` under `p.mu` (`session_pool_scaling.go:131-138`), `createSession`'s `reserved` defer releases it on any early return (`:172-179`), and the transfer at `:246-249` is atomic under `p.mu`. `onActive` deletes from `startingSessions` (`session_pool_lifecycle.go:265`). Any new failure branch in `createSession` must preserve the invariant `pendingStarts + len(startingSessions) + Ready = "in-flight scale-up capacity"`. - **`budget.Acquire` blocks; must run OUTSIDE `p.mu`.** Currently at `session_pool_scaling.go:181`, deliberately after the `defer reserved` block and before any `p.mu.Lock()`. Moving it under the lock deadlocks scale-up under budget exhaustion. - **Slow-vRPC is fire-and-forget.** `recordSlowVRpc` (`session_pool_debug.go:301`) appends into a mutex-guarded ring; it's on the vRPC return path but bounded by threshold gating and one small mutex. Do not add I/O, logging fanout, or channel sends here. - **`newTestPool` / bootstrap defaults.** Test helpers construct the pool without a `ClientConfigurationManager`, so the bootstrap defaults from `defaultPoolConfig()` (`session_pool.go:218-226`) are the only config those tests see. Real callers always get `UpdateConfig` synchronously on registration — verify by reading `NewSessionPoolImpl` end-to-end, not by trusting the bootstrap values. - **`Close` phase ordering.** Phase 4 (`poolCancel`) runs AFTER Phase 3 (`wg.Wait` on graceful closes) because Phase 2's `closeCtx` derives from `p.poolCtx`; swapping order strands in-flight graceful closes on a cancelled ctx. Phase 5 (`p.spawns.Wait`) blocks on every createSession goroutine's `WaitGoroutines` — that's why createSession blocks on `s.WaitGoroutines()` at `session_pool_scaling.go:272`. --- ## Guide 2 — mutianf-bot (automated reviewer) ### REAL HAZARDS to flag - **Re-entrant `p.mu` in pool methods called from `CheckoutSession`.** Anchor: `session_pool.go:235-310`. `p.mu` is dropped at `:255` before `PickAfe` / `Checkout` / `recordPickDecision` fire. Flag any newly-added helper called from that block that re-acquires `p.mu`, or any new method that reads `p.picker.Name()` without taking the name as a parameter (see the intentional parameter pattern at `session_pool_debug.go:366`). - **`budget.Acquire` under `p.mu`.** Currently correctly outside the lock at `session_pool_scaling.go:181`. `SessionThrottler.Acquire` blocks on the budget semaphore; calling it while holding `p.mu` would deadlock scale-up. Flag any code path that acquires `p.mu` before line `:181` or moves `Acquire` inside a `Lock`/`Unlock` bracket. - **`sync.Map` allocations on hit paths.** `bumpCloseReason` uses `Load` first, `LoadOrStore(k, new(atomic.Int64))` only on miss (`session_pool_lifecycle.go:102-111`) — this is the correct pattern. Flag any new `sync.Map.LoadOrStore(key, new(...))` call on a hot path that isn't gated by a preceding `Load` — that allocates on every hit. - **Waiter counter drift.** `waitersCount.Add(+1)` at `session_pool.go:291`, `Add(-1)` on both the `ctx.Done` branch (`:294`) and the `w.ready` branch (`:300`). Flag any new wake path, timeout branch, or early-return between `:291` and `:308` that doesn't decrement, and any new enqueue site that doesn't increment. Drift here corrupts the sizer's `PendingCount` input. - **Unbalanced `pendingStarts` / `startingSessions`.** Tick increments `pendingStarts` under `p.mu` at `session_pool_scaling.go:131-138`; `createSession`'s `reserved` defer at `:172-179` releases on early return; the transfer to `startingSessions` at `:246-249` is atomic; `onActive` deletes at `session_pool_lifecycle.go:265`; failed-start deletes at `session_pool_scaling.go:253-255`. Flag any new failure branch in `createSession` that returns without either the `reserved` defer or an explicit transfer/cleanup. - **Missing CAS on close-once flags.** `sessionsClosed.Add(1)` sits downstream of `s.poolCloseRecorded.CompareAndSwap(false, true)` at `session_pool_lifecycle.go:121`. Lifetime histogram sits downstream of `sh.closingRecorded.CompareAndSwap(false, true)` at `:187` and `:317`. `recordSessionClose` re-entry is dedup'd at `:190` and `:347`. Flag any new counter bump / histogram record that isn't gated by the corresponding CAS. - **Uncontrolled goroutine spawns.** Every pool-spawned goroutine must be tracked so `Close` phases 3/5/6 can drain: createSession workers use `p.spawns.Add`/`Done` under `p.mu` (`session_pool_scaling.go:137-138,149`); `spawnTickOnce` uses the same pattern (`session_pool_lifecycle.go:464-476`); the mid-Close onActive dispatch adds at `:274`. Session-owned goroutines (`readLoop`/`heartbeatLoop`) are tracked on `Session.loops` and drained via `WaitGoroutines`. Flag any bare `go func(){…}()` in these files with no WaitGroup entry. - **Hot-path lock churn.** `p.mu` is briefly held on the fast path only at `session_pool.go:239-241` and `:249-255`. `Stats` takes `p.mu` (`:362`). Flag any new `p.mu.Lock()` inserted into `Invoke` (`:465-559`) or the pre-park stretch of `CheckoutSession` outside the two existing points. ### FALSE-POSITIVE SUPPRESSION - **"Package name should be `transport`, not `internal`."** The directory is `bigtable/internal/transport/` but the `package` declaration is `package internal` (confirmed at `session_pool.go:20`). Package name is correct as-is; do NOT flag. - **Local variable shadowing an exported type** where the local name is idiomatic (e.g. `afeID` local vs `AfeID` type in `CheckoutSession`). Already resolved intentionally; do NOT re-raise variants. - **`goimports` / `gofmt` / column-alignment / trailing-newline nits.** CI (`goimports -l`, `gofmt -l`, `go vet`) already gates these. Bot echo is noise. - **Comments referencing PR #20213 / #20215 / #20224.** Stacked-PR context, not stale references; do NOT suggest removal. - **Test coverage complaints for `pool_sizer.go`, `afe_picker.go`, `session_list.go`, `session.go`, `session_vrpc.go`, `session_throttler.go`, `client_configuration_manager.go`, `default_client_config.go`.** All shipped in earlier PRs (#20213, #20215, #20224) with their own tests; out of scope here. - **"Missing error wrapping"** on internal-only calls where the caller already annotates via `fmt.Errorf("POOL %s ...: %w", ...)` or via `btopt.Debugf`. Do NOT suggest adding a second wrap. - **Retry loop / context propagation questions on `Session.Invoke`.** That's the Session layer (`session_vrpc.go`), out of scope for this PR. - **"Consider using `sync.RWMutex` instead of `sync.Mutex` on `p.mu`."** The pool holds `p.mu` for tens of nanoseconds at a time and never for read-heavy loops; the added atomic on `RLock`/`RUnlock` would cost more than it saves. Do NOT suggest. - **"Consider extracting anonymous goroutine into named function."** Style-only; do NOT suggest for the three ticker loops or the createSession worker. ### SCOPE BOUNDARY Comment ONLY on: - `bigtable/internal/transport/session_pool.go` - `bigtable/internal/transport/session_pool_lifecycle.go` - `bigtable/internal/transport/session_pool_scaling.go` - `bigtable/internal/transport/session_pool_debug.go` - `bigtable/internal/transport/session_snapshot.go` - `bigtable/internal/transport/session_pool_*_test.go` - `bigtable/internal/transport/session_snapshot_test.go` Do NOT comment on additions to: - `session.go` / `session_vrpc.go` (WaitGoroutines / closeError additions — vetted) - `connpool.go` (`ChannelPickHintInto` helper — vetted) - `afe_picker.go` (`defaultAfeRandomSubsetSize` constant — vetted) - `debug_tracer.go` (3 new tags — vetted) These are supporting scaffolding, already reviewed by the 3 subagent reviewers in this stack. Only re-raise if something looks actively unsafe. ### EFFORT SCALING - ~4.9k LOC across 12 files. Do NOT paginate uniformly. - **First pass — the 4 hot source files, in this order:** 1. `session_pool.go` (559 LOC) 2. `session_pool_lifecycle.go` (538 LOC) 3. `session_pool_scaling.go` (311 LOC) 4. `session_pool_debug.go` (416 LOC) - **Second pass ONLY if a first-pass finding needs corroboration:** `session_snapshot.go` (594 LOC, mostly type defs), and the tests. Tests use `newTestPool`, which skips config wiring — do NOT flag bootstrap defaults on tests as if they were production paths. - If a first-pass finding is a real hazard from the list above, cite the file:line and the exact anchor pattern it violates. Do not file speculative "consider" comments.github.com-googleapis-google-cloud-go · 683eda8c · 2026-07-28
- 3.2ETVfeat(bigtable): add Session lifecycle (Start, Close, ForceClose, readLoop, heartBeatLoop) (#20215) ## Summary Third and final PR in the Session core stack. Adds the lifecycle orchestration that drives a Session from \`Start\` through teardown, plus the \`readLoop\` that dispatches server frames to the vRPC handlers landed in #20213. **Stacks on** #20213 (Session vRPC dispatch + slot lifecycle). Deletes the minimal \`ForceClose\` stub that PR shipped, replacing it with the full lifecycle-shaped body. ### What lands **New file: \`session_lifecycle.go\`** (~640 LOC) - \`Session.Start(ctx, OpenSessionRequest)\` — transitions New→Starting, Sends the OpenSession frame, fires \`onStart\`, spawns \`readLoop\` + \`heartBeatLoop\`. Wraps a failed \`Send\` as \`codes.Unavailable\` so retry plumbing treats pre-wire OpenSession loss the same as any other transport-side loss. - \`ForceClose\` — full body: \`transitionTo(Closed)\` → \`setCloseReason\` → \`notifyClosing\` (once) → \`cancelActiveRPCs\` → \`signalQuiescent\` → \`notifyClosed\`. - \`Close(ctx, CloseSessionRequest)\` — graceful drain: Ready→Closing, waits on \`quiescent\`, sends CloseSession, transitions to WaitServerClose, arms the pool's stuck-session sweep to eventually ForceClose if the server never confirms. - \`notifyClosing\` / \`notifyClosed\` — once-guarded hook dispatchers with the strict \`onClosing\`-precedes-\`onClose\` ordering enforced. - \`readLoop(ctx)\` — Recv-loop that dispatches every SessionResponse variant via \`handleSessionResponse\`, drives \`handleClose\` on stream termination, records \`msgsRecv\` counters + resets heartbeat deadline on every recognized frame. - \`handleSessionResponse\` — dispatch switch to \`handleOpenSession\` / \`handleVRPCResponse\` / \`handleErrorResponse\` / \`handleSessionParameters\` / \`handleGoAway\` / \`handleSessionRefreshConfig\`. Heartbeat frames reset the deadline; unknown-payload branch records a debug tag but doesn't reset (so a misbehaving server can't keep the watchdog satisfied with junk). - \`handleOpenSession\` — Starting→Ready transition, PeerInfo extract from the bidi header (synchronous, matches Java's onHeaders synchrony), fires \`onActive\`. - \`handleGoAway\` / \`handleClose\` / \`handleErrorResponse\` / \`handleSessionParameters\` / \`handleSessionRefreshConfig\` — protocol-level handlers. - \`heartBeatLoop\` — Timer + \`heartbeatWake\` reactive-wake pair. Only enforces the deadline while a vRPC is in flight (idle sessions legitimately receive no server heartbeats). See SESSION_SPEC.md #7. - \`peerInfoExtracter\` — parses the \`bigtable-peer-info\` header, stamps \`s.peerInfo\` atomically before \`onActive\` fires. - \`closeReasonLabel\` / \`closeReasonToCause\` — CloseSessionRequest.Reason → string / sentinel error mapping for close-reason attribution. **New file: \`session_lifecycle_test.go\`** (~660 LOC) — 30+ tests covering Start, ForceClose, Close, readLoop, handleSessionResponse dispatch, handleOpenSession + peerInfo parsing, handleGoAway, handleClose, handleErrorResponse (rpc_id=0 harmlessly drops via routeVRPCFrame guards), heartBeatLoop (reactive wake, idle-gate, missed-heartbeat ForceClose). **Edits to \`session_vrpc.go\`** - Delete the minimal \`ForceClose\` stub introduced in the prior PR — the full body lives in \`session_lifecycle.go\` now. **Edits to \`session_test.go\`** - \`hookCounts\` helper (start/active/close callback counters). - \`setSlotForTest\` helper (seed the in-flight slot from tests). ### Stack 1. #20211 — Session debug surface 2. #20213 — Session vRPC dispatch + slot lifecycle 3. **This PR** — Session lifecycle ## Test plan - [x] \`go build ./internal/transport/\` passes - [x] \`go vet ./internal/transport/\` clean - [x] \`go test ./internal/transport/ -count=1 -short -timeout 90s\` passes — 30+ new lifecycle tests plus all pre-existing testsgithub.com-googleapis-google-cloud-go · b9e53c62 · 2026-07-27
- 3.1ETVfeat(bigtable): add SessionClient + SessionTable + lazyPool (#20228) ## Summary Third of five PRs porting the session-pool infrastructure from `feat/bigtable-sessionz-debug` into upstream. Introduces `internal/session/` — a proto-native SessionClient + SessionTable API sitting on top of PR-2's SessionPoolImpl. - `internal/session/api.go` — public interfaces (`ChannelPool`, `Config`, `SessionClient`, `SessionTableAPI`, `DebugAccess`). - `internal/session/client.go` — `SessionClient` impl: dedicated channel pool (no primer), `ClientConfigurationManager` wiring, `OpenSessionTable` / `OpenAuthorizedView` / `OpenMaterializedView` factories that mint lazily-opened per-resource pools keyed by `{resource, permission}`. - `internal/session/table.go` — `SessionTable` impl. Two `*lazyPool` (read + write); MV is read-only (write pool nil, `MutateRow` returns `ErrWriteNotSupported`). `stampAttempt` sources per-attempt `cluster_id` / `zone_id` / peer fields from typed `InvokeResult.ClusterInfo` and `InvokeResult.PeerInfo` per CLIENT_SIDE_METRICS_SPEC #1. - `internal/session/lazy_pool.go` — `Invoker` + `SessionPool` interfaces + open-on-first-use lazy wrapper. Failed opens are NOT cached; the next call retries. - `internal/session/debug.go` — `DebugAccess` impl surfacing pool snapshots for sessionz/loadz/channelz/configz. ### Transport additions to support the above - `transport/debug_api.go` (new) — `SessionDebugProvider` / `ChannelDebugProvider` / `ConfigDebugProvider` interfaces + `ChannelPoolDebug` / `SessionRef` DTOs. Lives in transport (not bigtable) so `bigtable.Client` and `internal/session.SessionClient` can implement without an import cycle. - `transport/diverter.go` — `sessionPicks` / `classicPicks` counters, `DiverterSnapshot`, `Snapshot()`. - `transport/connpool.go` — `ChannelSnapshot` + `ChannelPoolSnapshot` type + method, `WithInstanceName` / `WithAppProfile` options for channelz labelling. - `transport/debug_tracer.go` — exported `DebugTag` + `RecordDebugTag` + `TagSessionAttemptNilClusterInfo` / `TagSessionAttemptEmptyClusterID` catalog constants. - `transport/session_descriptors.go` — `SessionType.ProtoName()` for human-readable pool identifiers. - `transport/direct_access_checker.go` — renames `newPingAndWarmDirectAccessChecker` → `NewPingAndWarmDirectAccessChecker` (constructor exported) and nil-guards `primer.Prime` so session-based clients can pass a nil primer. Session clients warm channels on-demand via `OpenSession`, not eagerly at pool-init. ### Lifecycle correctness `sessionClient.Close()` snapshots owned resources under `poolsMu` and releases the lock before running `Close` / `Shutdown` / `Cancel` calls, so a snapshot method holding `poolsMu` never deadlocks teardown. Post-Close Opens surface a distinct `ErrSessionClientClosed` sentinel rather than misleading `errReadPoolNil` / `ErrWriteNotSupported`. ## Stack - **PR-1** #20224 (sessionList) — merged - **PR-2** #20225 (SessionPoolImpl) — open; this PR stacks on it - **PR-4** (this PR) - PR-5 will follow with the bigtable-package integration + debugview. The diff on this PR includes PR-2's commits until #20225 merges into main. ## Test plan - [x] `go build ./internal/session/ ./internal/transport/ ./...` - [x] `go test ./internal/session/ -race -count=1 -short -timeout=180s` - [x] `go test ./internal/transport/ -race -count=1 -short -skip 'AfeLbSim|TestHighQpsSession' -timeout=240s` - [x] `gofmt -l ./internal/session/ ./internal/transport/` — clean - [x] `go vet ./internal/session/ ./internal/transport/` — clean - [x] Reviewed against the 4 behavioral specs (SESSION_SPEC, SESSION_CLIENT_SPEC, SESSION_POOL_SPEC, CLIENT_SIDE_METRICS_SPEC) and SESSION_COMPONENT_SPEC (boundary rules) — PASS.github.com-googleapis-google-cloud-go · ab2c96c3 · 2026-07-28
- 2.8ETVrefactor(bigtable): extract metrics tracer into internal/metrics for classic+session reuse (#20099) Summary 1. Decouple the metrics Tracer from gax invoker 2. Move metrics into internal package 3. Keep NoopMetricsTracer for aliasing.github.com-googleapis-google-cloud-go · 2b621805 · 2026-07-08
- 1.3ETVtest(bigtable): end-to-end session/vRPC integration tests via bufconn fake server (#20272) ## Summary Ports 3 test files forward from a downstream feature branch, scoped to what upstream's session data plane already supports. **Zero production change** — pure test surface. ## Files - \`integration_session_fake_server_test.go\` — in-process fake BigtableServer with OpenTable / VirtualRpc / PingAndWarm / GetClientConfiguration and configurable per-attempt error injection, vRPC-stall queue, PeerInfo rotation, SessionParameters keepalive. - \`integration_session_harness_test.go\` — wires the fake at a bufconn-backed \`*grpc.Server\`, dials via \`WithContextDialer\` (not \`WithGRPCConn\` — see harness comment for the DirectAccessChecker double-close hazard), and waits for the initial \`ClientConfigurationManager\` poll to flip the Diverter to \`SessionLoad=1.0\`. - \`integration_session_test.go\` — 16 tests covering ReadRow / Apply happy paths, request-shape assertions (Deadline, Metadata), retry classifier (server-directed vs bare), session reuse, multi-table isolation, ctx cancel / DL exceeded, concurrent load, missed-heartbeat reactive watchdog, Client Close → CloseSession exchange. ## Adaptations vs. the downstream original - Dropped \`ClientConfig.EnableSessionPool / SessionPoolMin / Max\` from the harness — upstream constructs session unconditionally when \`preDialed=false\`, pool sizing is driven by \`GetClientConfiguration\`. - Dropped \`TestIntegration_SessionVRpc_PeerInfoParsedIntoAfeID\` — depends on \`client.SessionDebug()\` (debugview surface) not yet upstream. - \`TestIntegration_SessionVRpc_HeartbeatWatchdog\` — trimmed the \`SessionDebug.CloseReasons\` cross-check; retry-succeeded + zero \`CloseSession\` frames still prove the watchdog fired via \`ForceClose\`. - Replaced \`TestIntegration_SessionVRpc_RetryExhaustion\` with \`TestIntegration_SessionVRpc_BareServerResultNotRetried\`. The original's premise (MaxAttempts caps retries on a queued Unavailable) doesn't hold on the session path — bare status is classified \`StateServerResult\` and never retried; server-directed \`RetryInfo\` bypasses MaxAttempts entirely. The rewrite pins upstream's actual behavior. ## Test plan - [x] \`go build ./...\` clean - [x] \`go vet ./...\` clean - [x] \`goimports -l\` clean - [x] 16 new tests green under \`-race\` in ~2s - [x] Full \`bigtable\` package sweep green modulo two pre-existing flakes (\`TestIntegration_NewClientWithEmulatorHost\` on upstream/main; \`TestSessionTableCache_TTLSweepEvictsIdle\` passes in isolation)github.com-googleapis-google-cloud-go · 0d0fa0a6 · 2026-07-30
- 1.3ETVfeat(bigtable): add per-AFE sessionList for the two-tier session pool (#20224) ## Summary First of five PRs porting the session-pool infrastructure from `feat/bigtable-sessionz-debug` (which powers the recycle-repro fleet) upstream. Stacks on #20215 (Session lifecycle, now merged). ### What lands **New file: `session_list.go`** (~600 LOC) — the per-pool sessionList data structure that groups sessions by the AFE (Application Front End) their handshake landed on. Consumed by the two-tier picker (K-choice-over-AFEs → dequeue-idle-session) in a follow-up PR. Key types: - `AfeID` (int64) — AFE identifier from the server's PeerInfo header at session-open. 0 is the sentinel "unknown" bucket for handshakes that did not carry a peer-info header. - `AfeSnapshot` — value-typed view of an afeHandle for pickers to score without holding sl.mu (Checkout re-resolves by ID; no *afeHandle escapes). - `AfeSnapshotRow` — debug-UI row emitted by `sessionList.Snapshot()` (consumed by afez/sessionz in a later debug PR). - `afeHandle` (unexported) — per-AFE bucket: FIFO idle queue, refCount (idle + inFlight + closing), two PeakEwma trackers. - `SessionHandle` — pool bookkeeping wrapper around Session. Carries `inExpectedCount` (I5 guard against WaitServerClose retry storm) and `activated / closingRecorded / closeRecorded` dedup flags for the pool's per-session hook chain. - `sessionList` (unexported) — the state machine, guarded by `sl.mu`. The state model documents **six invariants (I1-I6)** that every method preserves: ``` I1 inExpectedCount ⇒ handleToAfe[sh] != nil I2 readyCount == count of inExpectedCount handles I3 afesWithReady == {afe : len(afe.sessions) > 0} I4 afe.refCount == count of handleToAfe entries pointing at afe I5 sh in afe.sessions ⇒ handleToAfe[sh]==afe AND inExpectedCount I6 refCount-- only on OnSessionClosed (Closing keeps slot warm) ``` Lock order: `sl.mu` ONLY. `RecordVRpcOutcome` deliberately drops `sl.mu` between the map lookup and the `PeakEwma.Update` so the hot vRPC-outcome path doesn't serialize on it. Consolidates AFE types (`AfeID` + `AfeSnapshot`) that previously lived in `afe_snapshot.go` — sessionList now owns all AFE-bucket concepts. Deletes `afe_snapshot.go`. **New file: `session_list_test.go`** (~770 LOC) — I1-I6 coverage plus per-method tests (OnSessionStarted / Checkout / ReleaseToPool / OnSessionClosing / OnSessionClosed / RecordVRpcOutcome / ReadyAfes / Snapshot / AllHandles / Prune) and a concurrency stress test covering the documented lock-drop path in RecordVRpcOutcome. **Edits to `debug_tracer.go`** — three new tag constants for sessionList bookkeeping violations (all unreachable-under-invariants, kept as belt-and-suspenders): - `tagSessionListStartedNilSession` - `tagSessionListRefcountUnderflow` - `tagSessionListReadyCountUnderflow` **Edit to `session.go`** — one-line comment retarget on `AfeID()` (type now lives in `session_list.go`, not the deleted `afe_snapshot.go`). ### Stack 1. #20211 — Session debug surface (merged) 2. #20213 — Session vRPC dispatch (merged) 3. #20215 — Session lifecycle (merged) 4. **This PR** — sessionList (PR-1 of 5) 5. Next — SessionPoolImpl core, pool_lifecycle, pool_scaling, pool_debug ## Test plan - [x] `go build ./internal/transport/` passes - [x] `go vet ./internal/transport/` clean - [x] `go test ./internal/transport/ -race -count=1 -short -timeout=120s` passes — 20+ new sessionList tests plus all pre-existing.github.com-googleapis-google-cloud-go · dbf0c3f3 · 2026-07-27
- 1.2ETVfeat(bigtable): TTL-on-idle cache for per-resource session.TableAPI (#20263) ## Summary Wraps `Client.sessionTables` (a bare `map[string]session.TableAPI` today) in a `sessionTableCache` that evicts entries idle for more than a TTL (default 1 h). The wrapper `sessionTableHandle` **IS the cache entry** and implements `session.TableAPI`, so every `ReadRow` / `MutateRow` touches lastAccess automatically — no cooperation from `TableShim` needed. ### Before - Cache grew monotonically for the `Client`'s lifetime. - Once opened, a resource's session `TableAPI` stayed in the map even if the caller stopped touching it — its pools kept running, its server-side slots stayed occupied. - Bounded by _(resources ever opened)_, not _(resources currently in use)_. ### After - Idle entries evict after 1 h. - Cache size tracks active use. - Caller can also `Close()` the returned handle explicitly for immediate eviction. ## Design — caching handle (single-type approach) ```go type sessionTableHandle struct { api session.TableAPI key string cache *sessionTableCache lastAccessNano atomic.Int64 closeOnce sync.Once } ``` Both eviction paths (caller-initiated + TTL sweep) route through `handle.Close`, guarded by `closeOnce` so double-close from any combination of paths is safe. `cache.removeEntry(key, h)` deletes only if the map still holds THIS handle — protects against a concurrent Open that already replaced the slot. ## Client.Close ordering Close now runs in three phases: 1. `sessionTables.close()` — stops the sweeper, `Close()`s every remaining handle. 2. `sessionImpl.Close()` — drops the session backend's shared gRPC channels + `ConfigurationManager` poller. 3. `mPool.Close()` — drops the classic gRPC channels. ## Caveat, documented on the handle type `sessionTable.Close()` in `bigtable/internal/session/table.go:226-228` is a no-op today — pools tear down only when `session.Client.Close` fires. So the eviction wiring in this PR is complete on the bigtable side but **only frees session pools** once the session package grows real per-resource teardown. That's a follow-up on the session side. ## Depends on - **#20262** — session-backend wiring in `Client` (adds `sessionImpl` + `sessionTables` fields this PR replaces the map on). ## Tests (all under `-race`) - `TestSessionTableCache_HandleIsCacheEntry` — repeat `getOrOpen` on same key returns identical `*sessionTableHandle`. - `TestSessionTableCache_ReadRowTouchesLastAccess` — wrapper's `ReadRow` bumps `lastAccess`. - `TestSessionTableCache_CloseEvictsAndFires` — `handle.Close` removes from map + calls `api.Close`; second `Open` mints a fresh handle; double-close safe (`closeOnce` guards cache eviction, not `api.Close` — which fires each call, per that call's own contract). - `TestSessionTableCache_TTLSweepEvictsIdle` — advance `fakeClock` past TTL, wait for 1 ms sweeper, verify entries gone + `Close`d. - `TestSessionTableCache_TouchDefersEviction` — touched entry stays alive across multiple half-TTL advances. - `TestSessionTableCache_CloseEvictsAll` — `c.close()` shuts sweeper and `Close`s every remaining entry; idempotent. - All 10 pre-existing `TestOpen*` / `TestGetOrCreateSession*` tests still green (the cache is transparent to them). ## Test plan - [x] `go build ./...` clean - [x] `go vet ./...` clean - [x] `goimports -l` no output - [x] `go test ./bigtable/ -run 'TestOpen|TestGetOrCreateSession|TestSessionTableCache' -count=1 -race -v` — 16/16 passgithub.com-googleapis-google-cloud-go · 00b2a49d · 2026-07-30
- 1.1ETVfeat(bigtable): add ClientConfigurationManager (#19986) ## Summary - Add `ClientConfigurationManager` in `bigtable/internal/transport`: polls `GetClientConfiguration` on a fixed interval, parses the response into a typed `clientConfig`, and fans changes out to registered listeners. Supports RPC retry, validity-window fallback, and `Close()` shutdown that waits for in-flight polls. - Test suite covering construction, polling, listener fan-out, transient-error retry, and `Close` semantics.github.com-googleapis-google-cloud-go · 3a8f9270 · 2026-07-21
- 0.9ETVfix(bigtable): sessionTableHandle self-heals across cache eviction (#20296) ## Summary \`TableShim\` caches a \`*sessionTableHandle\` at Open time and holds it for the process lifetime. When \`sessionTableCache\`'s TTL sweeper evicts that handle (default 1h idle), subsequent \`ReadRow\` / \`Apply\` through the stale pointer hit a \`Close\`d pool and return wrapped \`btransport.ErrPoolClosed\`. That sentinel is \`codes.Unknown\` (not \`codes.Unimplemented\`), so \`InterceptUnimplemented\` does NOT fall back to classic. The user's \`*Table\` stays poisoned until process restart. ## Fix (Design C) Extract an iterative \`dispatch()\` helper on \`*sessionTableHandle\`: - \`Close()\` sets \`evicted atomic.Bool\` BEFORE \`api.Close()\` so future readers observe eviction before the underlying pool tears down. - \`ReadRow\` / \`MutateRow\` call \`dispatch()\`, which atomic-Loads \`evicted\` on the happy path (one atomic per RPC) and, on eviction, routes through \`resolveSuccessor\` — identity-checked \`removeEntry\` + \`getOrOpen\` — to install or find a live successor and dispatch there. Loops (not recursion) to survive a tight-TTL test where the fresh successor itself gets evicted mid-flight. - Cache is closed → \`resolveSuccessor\` returns nil → \`dispatch\` falls through to the original doomed handle → the surviving \`h.api\` surfaces the honest terminal error rather than looping. ## Design tradeoffs considered - **Per-RPC provider closure**: ~30 ns / RPC cache-map lookup on every call. Rejected — hot-path cost. - **Cache-side ErrPoolClosed catch**: leaks the transport-layer error sentinel into the cache/handle layer. Rejected — layering. - **Design C** (this PR): ~2 ns / RPC atomic Load on the happy path; ~30 ns Open-time cost once per table opened. ## Test plan - [x] \`TestSessionTableHandle_EvictedSelfHeals\` — post-\`Close\`, ReadRow / MutateRow succeed via a freshly minted successor. \`openFn\` re-invoked; cache holds the successor with distinct identity. - [x] \`TestSessionTableHandle_SweeperEvictionSelfHeals\` — the actual production trigger. Advances a fake clock past TTL, drives \`sweepOnce\`, asserts the held pointer still services RPCs. - [x] \`TestSessionTableHandle_EvictedReopenFailsGracefully\` — cache closed → ReadRow returns within a bounded 1 s deadline (no loop) and cache stays empty (no leaked successor). - [x] \`TestSessionTableHandle_ConcurrentEvictAndReadRace\` — \`-race\`: one goroutine spins \`Close\`+\`getOrOpen\` cycles while another spins \`ReadRow\` through the original held handle. Clean. - [x] \`TestSessionTableHandle_EvictionStormConvergesOnSingleInstalled\` — N=32 concurrent post-eviction RPCs converge on exactly one installed successor; every loser api is \`Close\`d. - [x] Existing \`TestSessionTableCache_*\` suite unchanged and passing. - [x] \`go test ./ -race -count=1 -short\` clean. Bug reference: internal audit finding #6 (\"TTL evict → ErrPoolClosed\").github.com-googleapis-google-cloud-go · 0dd98cd7 · 2026-08-03
- 0.8ETVfeat(bigtable): add Session struct + state machine (#20117) ## Summary Adds the Session struct and its atomic state machine. Scope trimmed to just `session.go` + `session_test.go` per reviewer feedback; observability (`sessionDebug` / `sessionTracer`) and `SessionHandle` land in follow-up PRs. - **`session.go` (~380 LOC)** — Session struct + lifecycle types: - `Stream` interface, `SessionHooks` (with `OnStart`/`OnActive`/`OnClosing`/`OnClose`), `vrpcResult` (three-way tagged union), `vrpcImpl`. - `Session` struct with atomic `state`, `activeRPC`, `peerInfo`, `refreshConfig`, heartbeat deadline fields, and quiescent-once channel. - `transitionTo(to, predicate)` CAS-plus-retry loop for state transitions; `isState` / `notState` predicate builders. - `signalQuiescent`, `LogName`, `State`, `PeerInfo`, `AfeID`, `RefreshConfig` accessors. - `sessionErr` + `unavailable(cause, format, args...)` — wraps `codes.Unavailable` with a sentinel cause so `status.Code(err)` and `errors.Is(err, sentinel)` both work. - `AfeID` type (exported per go vet). - Sentinel errors: `ErrSessionNotActive`, `ErrUnavailableHeartBeatMissed`, `ErrUnavailableGoAway`, `ErrUnavailableSessionError`. - `lastStateChangeNano` inlined directly on Session so `transitionTo` can stamp it without depending on the (follow-up) debug surface. - **`session_test.go` (~320 LOC)** — 14 tests covering: defaults, CAS transitions (happy + rejected + concurrent), predicate builders, quiescent channel, AfeID resolution, RefreshConfig accessor, vrpcResult union, unavailable() wrapping, and SessionHooks dispatch. ## What was dropped from the prior revision `session_debug.go` (+ test), `session_tracer.go` (+ test), `session_handle.go` (+ test) all move to follow-up PRs. Two things kept locally: - `AfeID` type declaration stays here (referenced by the follow-up picker + the sessionDebug type). - `lastStateChangeNano` inlined as a direct field on `Session` rather than embedded via `sessionDebug`, so `transitionTo` can stamp it standalone. ## Test plan - [x] `go test ./bigtable/internal/transport/ -run 'TestSession|TestVrpc|TestUnavailable|TestIsState|TestNewSession' -count=1 -short` → 14/14 pass locally. - [x] `go build ./bigtable/internal/transport/` clean. - [x] `go vet ./bigtable/internal/transport/` clean. - [x] `golint bigtable/internal/transport/session.go bigtable/internal/transport/session_test.go` clean. - [ ] CI green.github.com-googleapis-google-cloud-go · 09acbb37 · 2026-07-23
- 0.8ETVfeat(bigtable): add PoolSizer for server-driven session pool capacity (#20189) ## Summary Adds `PoolSizer`, a stateless snapshot-driven calculator that produces scale-up / scale-down / dead-band / no-stats decisions from current pool metrics + server config. Standalone (no callers on main yet) so the type + tests can land ahead of the upcoming `SessionPoolImpl` consumers. - **`PoolStats`** — snapshot of ready / starting / in-use / pending counts at a single instant. - **`StatsFetcher`** — closure indirection so the sizer never reaches into pool internals; the pool passes a getter, the sizer calls it once per `Decide`. - **`PoolSizer.Decide`** — returns a full `ScaleDecision` trace: every input, every intermediate (`EffectivePending`, `SessionsInUse`, `IdleHeadroom`, `DesiredRaw`, `DesiredCapacity`, `ImmediateCapacity`, `EventualCapacity`), and the final `Delta` + `Branch`. Operators consume the trace on debug pages to answer "why did the sizer choose this" without re-running the arithmetic. - **`PoolSizer.UpdateConfig`** — driven by the server-config listener. Guards the same way the constructor does: non-positive `Headroom` falls back to `0.10`; zero `NewSessionQueueLength` is ignored (would divide-by-zero the pending calculation). - **Passive-shrink contract**: scale-down `Delta` is **advisory only**. The client never proactively kills sessions; the pool's `OnClose` reads the delta and lets the pool shrink by one per naturally-closed session. This design cannot oscillate. - **`MinIdleSessions` floor** (default 1) prevents cushion-collapse — an idle pool with `InUseCount==0` would otherwise want zero sessions and starve cold-start. - **Fail-safe `no-stats` branch** when the fetcher returns nil (pool not started yet). ## Test plan - [x] `go build ./bigtable/...` - [x] `go test ./bigtable/internal/transport/ -run '^TestPoolSizer' -count=1 -race` — 13 tests pass (1.0s) - [x] `gofmt -l bigtable/internal/transport/pool_sizer*.go` — clean - [x] `go vet ./bigtable/internal/transport/` — clean Test coverage: - `no-stats` branch on nil fetcher - Cold-start scale-up (clamp to `MinSessions`) - Dead-band absorbed by starting sessions - Scale-down advisory-only (negative delta, never zero-cross) - `MaxSessions` clamp under extreme demand - `MinIdleSessions` floor prevents cushion-collapse - `EffectivePending = ceil(PendingCount / NewSessionQueueLength)` (table-driven) - Constructor + `UpdateConfig` both normalize non-positive `HeadroomPct` to `0.10` - `UpdateConfig` live-swap takes effect - Zero `NewSessionQueueLength` from server ignored (divide-by-zero guard) - Every `ScaleDecision` intermediate populated from one evaluation - `GetScaleDelta` matches `Decide().Delta` - Concurrent `Decide` + `UpdateConfig` under `-race` (single mutex)github.com-googleapis-google-cloud-go · 57ebbeb8 · 2026-07-22
- 0.7ETVfeat(bigtable): add getClientConfigDirectAccessChecker for session pools (#20209) ## Summary Adds a session-pool sibling to `pingAndWarmDirectAccessChecker`. Session channel pools do not use PingAndWarm — they warm channels via the `OpenSession` handshake on each newly-opened stream (see #20208 — `NoOpChannelPrimer`). Passing a NoOp primer into the classic checker would leave `isALTSConn` unset, so the ALTS check would always fail for session pools. `getClientConfigDirectAccessChecker` runs the same `CheckCompatibility` flow (dial → probe → ALTS check → success-metric or async investigation), but issues `GetClientConfiguration` as the probe RPC — the same verb session pools already talk on the wire. ## Changes - **`direct_access_checker.go`** (modified): extract two shared helpers from `pingAndWarmDirectAccessChecker` so both checkers can use them without duplication. - `investigateDirectAccessFailure(logger, reportFailure, probeSingle, originalErr)` — the GCE-environment precondition walk, now taking a `probeSingle` callback so each checker plugs in its own RPC-verb probe. - `newAltsProbeChannel(ctx, targetEndpoint)` — the ALTS + oauth + authority-override dial used by the single-endpoint investigation probe. Behavior unchanged. - `pingAndWarmDirectAccessChecker.investigateFailure` / `.probeSingleEndpoint` become thin wrappers over the shared helpers. Existing behavior preserved. - **`direct_access_checker_getclientconfig.go`** (new): `getClientConfigDirectAccessChecker` struct, constructor, `CheckCompatibility`, `probeGetClientConfig` (the compatibility probe), `probeSingleEndpoint` (the single-endpoint investigation probe), and `recordProbePeer` — the ALTS + IP-protocol side-effect helper that mirrors what `BigtableConn.Prime` does for PingAndWarm. - **`direct_access_checker_getclientconfig_test.go`** (new): 7 tests covering interface satisfaction, dialer identity, dial-failure short-circuit, and the four IP/ALTS observation branches of `recordProbePeer`. ## Test plan - [x] \`go test ./bigtable/internal/transport/ -run 'GetClientConfigDirectAccess|RecordProbePeer' -count=1 -short\` → 7/7 pass. - [x] \`go test ./bigtable/internal/transport/ -count=1 -short\` (full transport suite, ~200 tests, 42s) → all pass, verifying the pingAndWarm refactor is behavior-preserving. - [x] \`go build\` / \`go vet\` / \`golint\` all clean. - [ ] CI green. ## Follow-up A subsequent PR will add a session-pool factory that wires this checker alongside `NoOpChannelPrimer` (from #20208) and the `ClientConfigurationManager` polling loop.github.com-googleapis-google-cloud-go · 3b8d30ad · 2026-07-28
- 0.7ETVfix(bigtable): real per-resource pool teardown on sessionTable.Close + cache close-race gate (#20264) Fixes two related latent bugs in the session data plane: 1. **sessionTable.Close was a no-op** — the interface godoc at `bigtable/internal/session/api.go:45-48` promises to release the resource's read + write session pools; the implementation returned nil. Pools were reclaimed only by `sessionClient.Close`, so `bigtable.Client`'s TTL cache could evict a handle without freeing the underlying pool + streams + goroutines. 2. **sessionTableCache close-race (audit finding #5)** — a slow-path openFn straddling `sessionTableCache.close()` would install a fresh handle into a cache the sweeper had already stopped clearing. Zero-impact while sessionTable.Close was a no-op; a per-race pool leak once teardown becomes real. ## Design Real teardown routed through symmetric release closures: - `sessionClient.releaseSessionPool(key)` — under `sessionPoolsMu`, delete the entry then `unregister()` + `pool.Close()` outside the lock (matches the snapshot-under-lock pattern in `Close`). - `buildLazyReleaser(key)` — sibling to `buildLazyOpener`; returns a `func() error` closure for a specific poolKey. - `sessionTable.Close` — invokes closeRead + closeWrite, joining errors via `errors.Join`. Nil-safe for materialized views' missing write side. No refcount inside sessionClient. Rationale: `bigtable.Client`'s sessionTableCache dedupes handles per fully-qualified resource name, so at-most-one sessionTable per resource per Client at any moment — the cache is the 'refcount of at-most-1'. Doc on `sessionTable.Close` names the invariant; a future caller bypassing the cache would need to add a refcount then. Paired cache guard: `sessionTableCache` gains a `closed bool` under `c.mu`, flipped by `close()`. `getOrOpen`'s slow path re-checks it before insert and, if set, releases the freshly-opened api and returns nil so `TableShim` falls back to classic. Prevents the finding-#5 leak. ## Tests New unit tests (all pass under `-race`): - `TestSessionTable_Close_CallsBothReleasers` - `TestSessionTable_Close_NilWriteReleaserOK` (materialized view) - `TestSessionTable_Close_JoinsErrors` - `TestSessionTable_Close_ReleasersIdempotent` - `TestReleaseSessionPool_AfterClientClose_NoOp` - `TestReleaseSessionPool_MissingKeyNoOp` - `TestReleaseSessionPool_RemovesEntryAndInvokesUnregister` - `TestSessionTableCache_ClosedGate_SlowPathInsertNoLeak` ## Drive-by The pre-existing `closeCountingTable` test helper races between the sweeper goroutine's `*counter++` and the test-goroutine's read. Switched `*int` to `*atomic.Int32` so the full-package `-race` sweep is green (fires on the base branch too — this was blocking my race-stress verification). ## Test plan - [x] `go build ./...` and `go vet ./...` clean - [x] `go test -race -count=1 -short -timeout=120s ./ ./internal/session/ ./internal/transport/` — all green - [x] `TestSessionTable_Close*` and `TestReleaseSessionPool*` — all pass - [x] `TestSessionTableCache_ClosedGate_SlowPathInsertNoLeak` — reproduces the race deterministically, passes with the fix - [x] Sandbox smoke against sushanb-uc1 via `CBT_RUN_SANDBOX=1 CBT_FORCE_SESSION=true`: Table + AV round-trip on both classic and session paths ## Stack Stacked on #20263 (session_table_cache) which is stacked on #20262 (session.Client wiring into bigtable.Client Open*). Both must land first, or this PR must be rebased onto main after they merge.github.com-googleapis-google-cloud-go · 599aea9e · 2026-07-30
- 0.7ETVfeat(bigtable): add ChainInterceptors and RetryingVRpc for vRPC pipeline (#20185) ## Summary Builds on the vRPC primitives merged in #20116 by adding the two building blocks that consumers need to actually run interceptor pipelines with retries: - **`ChainInterceptors`** (`internal/transport/chain.go`) — composes multiple `Interceptor`s around a base `Handler`. First interceptor in the list is outermost, so it sees setup before and teardown after the rest. - **`RetryingVRpc`** (`internal/transport/retrying.go`) — an `Interceptor` that retries failed virtual RPCs with exponential backoff. Default classification is Java-parity: - `StateUncommitted` → always retry (server never saw the frame). - `StateTransportFailure` → retry only when `Idempotent` is true (server may have applied). - `StateServerResult` → retry only if the server attached `errdetails.RetryInfo`; a bare server-explicit error (even `Unavailable` / `Aborted` / `DeadlineExceeded`) does NOT retry without explicit server go-ahead. - Callers with bespoke policies can set `ShouldRetry` to override the default entirely. Per-attempt tracing goes through `VRpcTracer`; the previous attempt's error is tagged onto the next attempt's context via `WithPrevAttemptErr` for downstream metrics/logging. ## Test plan - [x] `go build ./internal/transport/...` - [x] `go vet ./internal/transport/...` - [x] `go test ./internal/transport/... -run 'Retrying|Chained|Interceptor' -count=1 -race -timeout=90s` — 12 new tests pass: - `TestChainedInterceptors_Success` — interceptor call ordering (outer→inner→base→inner→outer) - `TestRetryingVRpc_SuccessOnRetry` — recovers on attempt 3, tracer sees all 3 starts/completes - `TestRetryingVRpc_NonRetryableError` — `InvalidArgument` aborts after 1 attempt - `TestRetryingVRpc_HonorServerRetryDelay` — server `RetryInfo` overrides client backoff - `TestRetryingVRpc_MaxAttemptsExceeded` — stops at `MaxAttempts` - `TestRetryingVRpc_NonGrpcError` — raw Go errors are non-retryable - `TestRetryingVRpc_UncommittedAlwaysRetries` — `StateUncommitted` retries even when `Idempotent=false` - `TestRetryingVRpc_TransportFailureIdempotent` — `StateTransportFailure` retries with `Idempotent=true` - `TestRetryingVRpc_TransportFailureNonIdempotentNoRetry` — no retry when `Idempotent=false` - `TestRetryingVRpc_ServerResultNotRetriedByDefault` — bare `ServerResult` never retries (5 codes) - `TestRetryingVRpc_ServerDeadlineExceededNoRetryByDefault` — server `DEADLINE_EXCEEDED` alone is not retried - `TestRetryingVRpc_ServerDeadlineExceededRetriesWithRetryInfo` — `DEADLINE_EXCEEDED` + `RetryInfo` is retried - [x] `goimports -d` clean - [x] `golint` cleangithub.com-googleapis-google-cloud-go · c7a832aa · 2026-07-22
- 0.6ETVfeat(bigtable): add sessionTracer for per-Session lifecycle + vRPC metrics (#20190) ## Summary Adds `sessionTracer`, the OTel metric-emitting companion to each Session. Standalone (no callers on `main` yet) so the tracer + tests can land ahead of the upcoming Session struct consumers. - **`InitializeSessionMetrics`** — `sync.Once`-guarded registration of four `Float64Histogram`s + the debug-tag counter. Safe to call multiple times with any provider (or `nil`); subsequent calls return the first-call error state. - **`sessionTracer`** — one instance per Session; holds only per-Session state (`startTime`, `opened` flag, `peerInfo`, `poolName`, `sessionType`). - **Four metrics registered:** | Metric | What it captures | |---|---| | `session.durations` | start → close latency | | `session.open_latencies` | start → OpenSession completion | | `session.uptime` | periodically-sampled age of active sessions | | `transport_latencies` | per-vRPC (e2e − backend) with fine-grain buckets | - `vrpcCloseState` label on `session.durations` mirrors Java's `SessionCloseVRpcState.find`: `{none, all_ok, all_error, some_ok}`. - `session_name` label is **pool-scoped (bounded cardinality)** — NOT the per-Session `logName` (unbounded). Documented in-file. - `recordTransportOverhead` gates on positive delta so a negative (e2e < backend) sample cannot corrupt the histogram. - Mutex discipline: `snapshot()` copies fields under lock; all allocating work (attribute builds, histogram `Record`) runs lock-free — debug/metrics MUST NOT block the hot path. ## Test plan - [x] `go build ./bigtable/...` - [x] `go test ./bigtable/internal/transport/ -run '^(TestSessionTracer|TestNewSessionTracer|TestVRpcCloseState|TestMsSince)' -count=1 -race` — 17 tests pass - [x] `gofmt -l bigtable/internal/transport/session_tracer*.go` — clean - [x] `go vet ./bigtable/internal/transport/` — clean Test coverage: - `InitializeSessionMetrics` idempotent + nil-provider safe. - `recordOpen` populates `session.open_latencies` with correct status label on OK vs error. - `recordClose` `vrpcs` label table (all four values). - `sampleUptime` emits for active session; skips zero startTime. - `recordTransportOverhead` positive-delta gate — zero and −3ms dropped, +5ms recorded, exact count == 1. - Nil-histograms no-op path (all recorders early-return without panic when Init was never called). - `newSessionTracer` defaults, `snapshot()` on nil vs set `peerInfo`, `vrpcCloseState` table, `msSince` sanity.github.com-googleapis-google-cloud-go · a4663459 · 2026-07-23
- 0.6ETVfeat(bigtable): add Session primitives (AttemptOutcome, vRPC ctx, msgtype) (#20116) ## Summary Adds the standalone types that the Session struct and its lifecycle / vRPC / debug halves all depend on. Each file is self-contained (no cross-references) so this PR compiles and passes tests on its own. - **`attempt_outcome.go`** — `AttemptState` (`StateUncommitted` / `StateTransportFailure` / `StateServerResult`), `tagErr`, `TagErr`, `ClassifyErr`. Models Java's `VRpc.VRpcResult.State` so the `RetryingVRpc` interceptor (later PR) can classify errors the same way as java-bigtable. - **`vrpc.go`** — ctx-metadata helpers (`WithVRpcMetadata`, `WithAttempt`, `VRpcAttempt`, `VRpcMethod`, `WithPrevAttemptErr`, `PrevAttemptErr`). `Session.Invoke` reads these from its ctx. - **`session_msgtype.go`** — `reqMsgType` / `respMsgType` enums + `classifyReq` / `classifyResp` helpers. Used by the debug surface + tracer to bucket Session request/response types. **Part 3a of 3 in the Session core sub-split** (a follow-up to the original Session core PR #20112, which is being reshaped into three thinner PRs). Stacks on #20115 (`metrics.TransportTypeName` export) and #20114 (debug tag counter); each of those can merge in any order. Sub-split order: 1. **3a — this PR:** Session primitives (~337 LOC). 2. **3b — TBD:** Session struct + tracer + debug surface + picker (~1.1k LOC). 3. **3c — reshaped #20112:** Session lifecycle + vRPC + tests (~2k LOC). ## Test plan - [x] `go build ./bigtable/...` - [x] `go test ./bigtable/internal/transport/ -count=1 -short` — passes (4.0s) - [x] `gofmt -l bigtable/internal/transport/` — clean - [ ] CI: presubmitgithub.com-googleapis-google-cloud-go · e1011e2d · 2026-07-10
- 0.6ETVfeat(bigtable): add Session debug surface (observability fields + methods) (#20211) ## Summary Introduces \`sessionDebug\` — a struct bundling the observability surface for a Session — and embeds it into \`Session\`. This is the first of three stacked PRs that layer per-Session behavior on top of the state machine from #20117. ### What lands **New file: \`session_debug.go\`** (~360 LOC) - Per-session atomic counters: \`okRpcs\`, \`errorRpcs\`, \`msgsSent\`, \`msgsRecv\`, per-frame-type breakdowns (\`msgsSentByType\`/\`msgsRecvByType\`), \`retries\`. - Debug event ring buffer (cap 64) + \`recordEvent\` / \`snapshotEvents\`. - Backend-latency histogram (256-sample ring) + \`recordLatency\` / \`snapshotLatencies\` + \`percentile\` helper. - Per-cluster response counts via \`sync.Map\` + \`recordCluster\` / \`snapshotClusters\`. - Close-reason attribution: \`setCloseReason\` / \`CloseReason\` (once-only stamp), \`poolCloseRecorded\` gate. - \`SessionTracer\` and \`log.Logger\` handles. - \`ChannelIndex\` + \`RemoteAddr\` accessors, \`peerInfoSummary\` helper. - \`SampleUptime\` / \`RecordTransportOverhead\` wrappers that delegate to the tracer. - Two \`SessionOption\` factories: \`WithSessionLogger\`, \`WithSessionPoolName\`. **Edits to \`session.go\`:** - \`lastStateChangeNano\` moves off \`Session\` onto \`sessionDebug\` (co-located with the rest of the observability plumbing). \`transitionTo\`'s swap-stamp continues to work through Go embedding. - \`sessionDebug\` embedded at the tail of the \`Session\` struct. - \`NewSession\` calls \`s.sessionDebug.init(sessionType)\` which sets the tracer, resets \`channelIndex\` to -1, and stamps \`lastStateChangeNano\`. ### Behavior on merged code paths **Zero.** Every field added by this PR has no writers on \`main\` today — the read-side accessors (\`HasOkRpcs\`, \`ErrorRpcs\`, \`MsgsSent\`, \`CloseReason\`, \`Retries\`, …) all return zero-values until the follow-up PRs wire the write sites. ### Stack 1. **This PR** — Session debug surface (\`session_debug.go\` + Session struct embed) 2. **Next** — \`session_vrpc.go\` (Session.Invoke + handleVRPC{Response,ErrorResponse} + cancelActiveRPCs; wires most of the write sites) 3. **Last** — \`session_lifecycle.go\` (Start/Close/ForceClose/readLoop/heartBeatLoop; wires the rest) ## Test plan - [x] \`go build ./internal/transport/\` passes - [x] \`go vet ./internal/transport/\` clean - [x] \`go test ./internal/transport/ -count=1 -short -timeout 90s\` passes (all existing tests; no new tests in this PR — \`session_debug_test.go\` lands with the vRPC follow-up when there are actual write sites to assert on)github.com-googleapis-google-cloud-go · d8d3e160 · 2026-07-24
- 0.6ETVfeat(bigtable): route Client.Open()-returned *Table through the Diverter (#20273) ## Summary Backward-compatible session-routing wiring on the bare \`*Table\` surface. When \`c.diverter\` is set, \`Apply\` and \`ReadRow\` on a \`*Table\` returned by \`Client.Open()\` now route through an internal \`TableShim\` so calls can be diverted to the session data path under the diverter's SessionLoad ratio. Return type stays \`*Table\`; every existing method keeps its signature and behavior. ## Why Callers that use \`Open()\` (BulkMutation, existing app code that holds \`*Table\` across many ops) previously never saw session routing — only \`OpenTable\` did. This closes the gap so the same \`*Table\` works for both paths. ## Design - **\`Table.divertible TableAPI\` field.** Nil for classic-only clients (no diverter) → gate short-circuits and the classic fast path runs unchanged. Populated by \`Open()\` when the client has a diverter. - **\`Table.Apply\` and \`Table.ReadRow\` gain a one-line gate at the top:** if \`t.divertible != nil\`, dispatch there; else fall through to the new \`applyClassic\` / \`readRowClassic\` helpers (pre-existing bodies extracted verbatim). - **\`tableImpl.Apply\` and \`tableImpl.ReadRow\` bypass the gate** — they call \`applyClassic\` / \`readRowClassic\` directly. Necessary because \`tableImpl\` is what \`NewTableShim\` wraps as its classic side; without the bypass the shim would recurse into itself. - **\`Open()\` calls \`c.buildDivertible(t, ...)\`** which returns a \`*TableShim\` wrapping a snapshot of \`t\` (with \`divertible\` nil-ed). \`buildDivertible\` returns nil when \`c.diverter\` is nil, so the zero-cost path stays intact. ## Cardinality The \`sessionTables\` cache map grows one entry per unique \`Open()\` call now (previously only per \`OpenTable\`). Cost per entry is ~110 B — a fully-qualified table name string + a \`*SessionTable\`. **No new sessions or streams open** — sessions still materialize lazily on first RPC per \`lazyPool.get()\`. See \`buildDivertible\` doc for the full cardinality analysis (three cardinalities in play, only one shifts). ## Files | file | change | |---|---| | \`bigtable/open.go\` | \`Open()\` wires \`t.divertible\`; new \`buildDivertible\` helper | | \`bigtable/table.go\` | \`Table\` gains \`divertible TableAPI\` field; \`tableImpl\` overrides \`Apply\` / \`ReadRow\` to bypass the gate | | \`bigtable/bigtable.go\` | \`Table.Apply\` / \`Table.ReadRow\` gain a divertible gate; classic bodies extracted into \`applyClassic\` / \`readRowClassic\` | ## Test plan - [x] \`go build ./...\` clean - [x] \`go vet ./...\` clean - [x] \`goimports -l\` clean - [x] full \`go test -race -count=1 -short -timeout=180s ./bigtable/ ./bigtable/internal/session/ ./bigtable/internal/transport/\` green modulo the two pre-existing flakes on \`upstream/main\` (\`TestIntegration_NewClientWithEmulatorHost\` — emulator-host resolver; \`TestSessionTableCache_TTLSweepEvictsIdle\` — passes in isolation)github.com-googleapis-google-cloud-go · 2b81c7dc · 2026-07-30
- 0.6ETVrefactor(bigtable): TableShim adopts proto-native session TableAPI (#20258) ## Summary Follow-up to #20257 (protoRowToRow), which now has its sole consumer. Swaps `TableShim`'s session backend from the classic `TableAPI` shape (row string + `bigtable.Row`) to the internal `session.TableAPI` shape (proto-native `*btpb.SessionReadRow{Request,Response}` + `*btpb.SessionMutateRow{Request,Response}`). `TableShim` now owns the proto ↔ public-types translation so the `internal/session` package can stay proto-native. ## Behavior **`ReadRow`** — parses `ReadOption` using the classic `makeReadSettings` shape (so filter + full-read-stats callback plumbing stays in one place), builds a `*btpb.SessionReadRowRequest`, calls `session.ReadRow`, feeds any `resp.Stats` through the `WithFullReadStats` callback, and converts `resp.Row` via `protoRowToRow`. **`Apply`** — conditional mutations (`CheckAndMutateRow`) always route to classic since the session vRPC has no `CheckAndMutateRow` equivalent. Non-conditional mutations build a `*btpb.SessionMutateRowRequest` and call `session.MutateRow`. **`useSession()`** — nil-safe on both `session` and `diverter`, so callers can wire a `TableShim` with `nil` session (as `buildDivertible` does in #20256) and every routing decision falls through to classic. **`ReadRows` / `SampleRowKeys` / `ApplyBulk` / `ApplyReadModifyWrite` always classic** — no session equivalent in the vRPC. ## API-visible signature change `NewTableShim`'s `session` parameter type becomes `session.TableAPI` (an internal-package interface). Existing in-repo callers pass `nil` for session today; nil is the zero value of any interface, so the change is source-compatible for those. External callers writing tests can mock `session.TableAPI` directly — the new test file has an example (`mockSessionTable`). ## Tests Replaces the `mockTableAPI`-as-session pattern with a proto-native `mockSessionTable`. New coverage: - `TestTableShim_ReadRow_RoutesByDiverter` — classic when `SessionLoad=0.0`; session when `SessionLoad=1.0`; classic fallback when session is nil even with `SessionLoad=1.0`; session error propagation (no automatic fallback to classic on failure). - `TestTableShim_Apply_ConditionalAlwaysClassic` — pins that conditional mutations bypass the session path. - `TestTableShim_Apply_NonConditionalRoutesByDiverter` — pins that non-conditional mutations follow the diverter. - `TestTableShim_ReadRows_AlwaysClassic`, `TestTableShim_SampleRowKeys_AlwaysClassic`, `TestTableShim_ApplyBulk_AlwaysClassic`, `TestTableShim_ApplyReadModifyWrite_AlwaysClassic` — pin the no-session-equivalent methods. - `TestTableShim_NilSession_AllMethodsFallBackToClassic` — the classic-only wiring path (what #20256's `buildDivertible` will use until the session backend lands). - `TestTableShim_SessionErrorNotRetriedOnClassic` — session-side failures surface as-is instead of silently falling back. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./...` clean - [x] `go test ./bigtable/ -run 'TestTableShim|TestProtoRowToRow' -count=1 -v` — all pass - [x] Live sandbox smoke against `autonomous-mote-782 / sushanb-uc1` — classic path via `client.Open(...).Apply/ReadRow` still succeeds end-to-end (session backend not exercised on this branch since it isn't wired yet) ## Depends on - #20257 (merged as `1297143a4f`) — `protoRowToRow` helper. ## Follows Consumed by a future PR that wires an actual `session.TableAPI` implementation from the `internal/session` package into `Client.buildDivertible` (see #20256 for the `buildDivertible` shape).github.com-googleapis-google-cloud-go · fdcefedc · 2026-07-29
- 0.5ETVfeat(bigtable): modularize Direct Access compatibility check (#19987) ## Summary - Extract the Direct Access compatibility decision out of `BigtableChannelPool` behind a new `DirectAccessChecker` interface so each channel pool factory can plug in its own strategy. - Today's only implementation, `pingAndWarmDirectAccessChecker`, preserves current behavior (PingAndWarm probe + ALTS check + async failure investigation + `direct_access/compatible` metric). The upcoming session pool factory will plug in a `GetClientConfiguration`-based checker (driven by `ClientConfigurationManager`) without touching the pool. - A small `disabledDirectAccessChecker` keeps the `direct_access/compatible{reason=manually_disabled}` reading when the env/config disables Direct Access — the env var check moves up to the factory so the pool no longer re-reads `CBT_ENABLE_DIRECTPATH`. ## Changes - **New** `internal/transport/direct_access_checker.go`: `DirectAccessChecker` interface, `pingAndWarmDirectAccessChecker`, `disabledDirectAccessChecker`, and the moved helpers (`xdsCdsURITemplate`, `checkIPPlumbing`, `checkKernelRoutes`, `probeSingleEndpoint`). - `internal/transport/connpool.go`: remove `directAccessDialer`, `directAccessFeatureFlagsMD`, `daEligibleGauge` fields, the `WithDirectAccessDialer` / `WithDirectAccessFeatureFlagsMetadata` options, the in-pool probe + investigation chain, the duplicate `CBT_ENABLE_DIRECTPATH` check, and the metric reporting helpers. Add `WithDirectAccessChecker`. `NewBigtableChannelPool` now just delegates to the checker. - `internal/transport/channel_pool_factory.go`: when `isDirectAccessEnabled(config)` is true, wire a `pingAndWarmDirectAccessChecker`; otherwise wire a `disabledDirectAccessChecker`. Both passed via `WithDirectAccessChecker`. - `internal/transport/connpool_test.go`: six `TestDirectAccessLogic` subtests updated to construct the appropriate checker. The env-disabled subtest becomes `DirectAccess_DisabledChecker` since the env-var gate has moved to the factory layer. ## Test plan - [x] `cd bigtable && go build ./...` - [x] `cd bigtable && go vet ./...` - [x] `cd bigtable && go test ./internal/transport/... -count=1` (full transport suite, including all six `TestDirectAccessLogic` subtests, `TestCreateAndStartManagedChannelPool*`, and `TestManagedChannelPool_Close`)github.com-googleapis-google-cloud-go · a25e93d2 · 2026-06-19