Sushan Bhattarai
90d · built 2026-09-08
Performance
What Sushan Bhattarai shipped in the selected window, measured in ETV, and how it compares with the 90 days before it.
Effective capacity
+18.1engineers
delivers like 19.1 (19.1x pre-AI)
Output (ETV)
24.9ETV
+1000.4% vs 2.3 prior
Features share
31.0%
−27.8 pp vs prior window
Fixes share
2.9%
−0.2 pp vs prior window
Work mix
31% Features10% Maintenance49.3% Tests6.8% Docs2.9% Fixes
59 commits over 90 days, ending 2026-09-08.
Daily performance
Daily ETV, stacked by Features, Maintenance, Tests, Docs and Fixes.
Repository spread
Where this developer's commits land. Concentrated work (top1 > 80%) vs polymath spread (top1 < 30%).
| Repo | Commits | ETV |
|---|---|---|
| google-cloud-go | 57 | 24.9 |
Most impactful commits
Top 10 by ETV in the last 90 days.
- 3.5ETVfeat(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
- 2.3ETVrefactor(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
- 2.3ETVfeat(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
- 2.0ETVfeat(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
- 0.9ETVfeat(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
- 0.9ETVtest(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
- 0.9ETVfeat(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
- 0.8ETVfeat(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.6ETVfeat(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.6ETVfeat(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