feat(bigtable): add Session debug surface (observability fields + methods) - #20211
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the Session struct by moving its observability and debugging state (such as counters, event ring, latency histograms, and tracers) into a newly introduced embedded sessionDebug struct defined in session_debug.go. This separation keeps the main session.go focused on the protocol state machine and vRPC dispatch while co-locating the observability logic. I have no feedback to provide.
This was referenced Jul 23, 2026
…hods) Introduces sessionDebug — a struct bundling every observability concern for a Session: per-session counters (okRpcs/errorRpcs/msgs*), event ring buffer, backend-latency histogram, per-cluster response counts, session tracer, debug logger, per-channel index, and close-reason attribution. Embedded into Session so existing bare-field access (s.tracer, s.okRpcs, s.recordEvent, ...) will compile once the vRPC and lifecycle follow-ups land. `lastStateChangeNano` moves off Session onto sessionDebug (co-located with the rest of the observability plumbing) — transitionTo's swap-stamp keeps working via embedding. NewSession wires the embed via s.sessionDebug.init(sessionType), which sets the tracer + channelIndex + lastStateChangeNano defaults. No behavior change on any merged code path — every field added by this PR has zero writers on main today. The Read-side methods (OkRpcs/ErrorRpcs/HasOkRpcs/HasErrorRpcs/MsgsSent/MsgsRecv/CloseReason/ Retries/StartedAt/RemoteAddr/ChannelIndex) all return zero-values until the follow-up session_vrpc.go PR wires the write sites. Prereq: PR googleapis#20117 (merged) — needs the Session struct + State machine. Follow-ups (in dependency order): session_vrpc.go, session_lifecycle.go.
CI vet fails on any file that goimports would reformat; goimports strips the trailing blank line after the last function. No behavior change.
sushanb
force-pushed
the
bigtable-session-debug
branch
from
July 23, 2026 22:46
2534c88 to
9c553f6
Compare
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 23, 2026
Introduces Session.Invoke and the one-in-flight vRPC dispatch that
routes server frames back to callers. Stacks on the sessionDebug bundle
(bigtable-session-debug); the counter/tracer/event writers land here.
New file: session_vrpc.go (~420 LOC)
- Session.Invoke — public entry point. Serializes claimSlot → Send →
awaitInvokeResult; deliver-vs-cancel branching lives in routeVRPCFrame.
- Slot lifecycle: claimSlot / drainSlot / markCancelled / activeVRPC —
slotMu-guarded, one vRPC in flight per session.
- handleVRPCResponse / handleVRPCErrorResponse + shared routeVRPCFrame —
Java-parity drain path with nil-active-VRPC + id-mismatch guards.
- deliver — writes to the caller's resultChan.
- cancelActiveRPCs — session-teardown path used by ForceClose /
handleGoAway / heartbeat watchdog.
- ForceClose — minimal port (transitionTo(Closed) → cancelActiveRPCs →
signalQuiescent). The full lifecycle-flavored ForceClose (setCloseReason
+ notifyClosing + notifyClosed + stream cleanup) lands with the
session_lifecycle follow-up.
Edits to session.go
- Add heartbeatWake chan struct{} field (cap-1 non-blocking) +
make(...) in NewSession — reactive watchdog wake channel.
- Add OnSlotDrained func() to SessionHooks + onSlotDrained() dispatcher.
Sole "session became free" signal; consumers wake pool waiters.
- Add Send(req) — mutex-guarded stream.Send wrapper; used by both this
PR's Invoke and the follow-up lifecycle Start.
- Add resetHeartbeatDeadline + wakeHeartbeatLoop — atomic-based reactive
wake pair; consumed by the follow-up lifecycle heartBeatLoop.
Edits to session_test.go
- Test fixtures: fakeStream / fakeDesc / newFakeStream / newRoundTripDesc /
makeActive / waitFor. Consumed by both this PR's TestInvoke*/TestHandleVRPC*
and the follow-up lifecycle tests.
- newTestSession signature change (stream, hooks); pre-existing 1-arg
call sites now use newDefaultTestSession shortcut.
Edits to debug_tracer.go
- tagSessionVRPCCancelledDrained — bookkeeping tag when a server response
drains a slot whose caller already ctx.Done'd.
Stack:
- Prereq: PR googleapis#20211 (bigtable-session-debug) — Session debug surface.
- Follow-up: session_lifecycle.go (Start, Close, readLoop, heartBeatLoop,
handleSessionResponse / handleOpenSession / handleGoAway / handleClose /
handleErrorResponse / peerInfoExtracter). Will extend ForceClose to its
full lifecycle-shaped body.
…ments
Two doc-comment references to java-bigtable ("Java-parity slot lifecycle"
in the SessionEvent kind list; "Matches java-bigtable's per-pool
SessionPoolInfo name" in WithSessionPoolName) rewritten to describe the
Go behavior directly. No code change.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 23, 2026
Introduces Session.Invoke and the one-in-flight vRPC dispatch that
routes server frames back to callers. Stacks on the sessionDebug bundle
(bigtable-session-debug); the counter/tracer/event writers land here.
New file: session_vrpc.go (~420 LOC)
- Session.Invoke — public entry point. Serializes claimSlot → Send →
awaitInvokeResult; deliver-vs-cancel branching lives in routeVRPCFrame.
- Slot lifecycle: claimSlot / drainSlot / markCancelled / activeVRPC —
slotMu-guarded, one vRPC in flight per session.
- handleVRPCResponse / handleVRPCErrorResponse + shared routeVRPCFrame —
Java-parity drain path with nil-active-VRPC + id-mismatch guards.
- deliver — writes to the caller's resultChan.
- cancelActiveRPCs — session-teardown path used by ForceClose /
handleGoAway / heartbeat watchdog.
- ForceClose — minimal port (transitionTo(Closed) → cancelActiveRPCs →
signalQuiescent). The full lifecycle-flavored ForceClose (setCloseReason
+ notifyClosing + notifyClosed + stream cleanup) lands with the
session_lifecycle follow-up.
Edits to session.go
- Add heartbeatWake chan struct{} field (cap-1 non-blocking) +
make(...) in NewSession — reactive watchdog wake channel.
- Add OnSlotDrained func() to SessionHooks + onSlotDrained() dispatcher.
Sole "session became free" signal; consumers wake pool waiters.
- Add Send(req) — mutex-guarded stream.Send wrapper; used by both this
PR's Invoke and the follow-up lifecycle Start.
- Add resetHeartbeatDeadline + wakeHeartbeatLoop — atomic-based reactive
wake pair; consumed by the follow-up lifecycle heartBeatLoop.
Edits to session_test.go
- Test fixtures: fakeStream / fakeDesc / newFakeStream / newRoundTripDesc /
makeActive / waitFor. Consumed by both this PR's TestInvoke*/TestHandleVRPC*
and the follow-up lifecycle tests.
- newTestSession signature change (stream, hooks); pre-existing 1-arg
call sites now use newDefaultTestSession shortcut.
Edits to debug_tracer.go
- tagSessionVRPCCancelledDrained — bookkeeping tag when a server response
drains a slot whose caller already ctx.Done'd.
Stack:
- Prereq: PR googleapis#20211 (bigtable-session-debug) — Session debug surface.
- Follow-up: session_lifecycle.go (Start, Close, readLoop, heartBeatLoop,
handleSessionResponse / handleOpenSession / handleGoAway / handleClose /
handleErrorResponse / peerInfoExtracter). Will extend ForceClose to its
full lifecycle-shaped body.
sushanb
commented
Jul 23, 2026
sushanb
left a comment
Contributor
Author
There was a problem hiding this comment.
igor-reviewer-agent
reviewed 93784b0 — most of this is inert plumbing so its mostly nits. main things worth another pass:
session_debug.go:38-104
this is a lot of new public surface (SetChannelIndex, ChannelIndex, RemoteAddr, HasOkRpcs/HasErrorRpcs/OkRpcs/ErrorRpcs, MsgsSent, MsgsRecv, Retries, StartedAt, SampleUptime, RecordTransportOverhead, CloseReason). package is internal so its not user-facing, but it still expands the intra-bigtable surface. can we mark these with a doc-comment convention (or keep them lowercase and expose a tiny SessionDebug accessor when sessionz needs them)? id rather grow the surface deliberately when a reader appears than land 12 accessors that are all zero-valued today.
session_debug.go:136-140
type SessionEvent struct {
At time.Time
Kind string
Message string
}nit, can Kind be a typed const? the four values are enumerated right above ("close"/"hb-missed"/"hb-alive"/"ctx-done") — a type SessionEventKind string with named constants would keep the writers honest when they land in the vRPC/lifecycle PRs. cheap now, painful to retrofit later.
session_debug.go:146-159
if len(s.events) >= maxSessionEvents {
copy(s.events, s.events[1:])
s.events = s.events[:len(s.events)-1]
}
s.events = append(s.events, ev)nit — you already do the proper wrap-index thing on latencySamples right below. this shifts N-1 entries on every append past cap. cap is 64 so its fine, but same-file inconsistency is a bit odd. either both or neither.
session_debug.go:179
return fmt.Sprintf("peer={afe=%x/%s/%s gfe=%x transport=%s}",why hex for AfeID and gfe? AfeID is a signed int64 elsewhere in the pkg and the debug UI treats it as decimal. I dont have strong feelings about it but the two renderings will diverge.
session_debug.go:252-261
func (s *Session) SetChannelIndex(idx int) { s.channelIndex.Store(int32(idx)) }
func (s *Session) ChannelIndex() int { return int(s.channelIndex.Load()) }two things: (a) silent int→int32 narrow on the setter, please just take int32; (b) doc says "called once at session construction" — if the only writer lives in this package, why is it exported? feels like it wants to be setChannelIndex set inline by NewSession or wired via a SessionOption.
session_debug.go:44,50,266-283
RemoteAddr / setCloseReason / closeReason all have readers here but no writers land until #20213/#20215. thats fine per the PR body, just flagging that anyone grepping this file today will see accessors that always return "". a one-line doc comment on the type saying "writers land in session_vrpc.go / session_lifecycle.go" would save future-me a grep.
session_debug.go:108-112
func (d *sessionDebug) init(sessionType SessionType) {
d.tracer = newSessionTracer(sessionType)
...
}nit, sessionType is redundant with the Session field; consider taking *sessionTracer or moving the newSessionTracer call up into NewSession and dropping the param. either shape is fine.
LGTM w/ nits — none of this is blocking, its all pre-write-site cleanup that costs less now than after the follow-ups land.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 23, 2026
…Loop, heartBeatLoop) 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 bigtable-session-vrpc. Stacks on bigtable-session-vrpc; deletes the minimal ForceClose stub that PR shipped, replacing it with the full lifecycle-shaped body. 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 update the deadline via resetHeartbeatDeadline; the unknown-payload branch records a debug tag but doesn't reset the deadline (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 - Add hookCounts helper (start/active/close callback counters). - Add setSlotForTest helper (seed the in-flight slot from tests). Stack: - googleapis#20211 — Session debug surface - googleapis#20213 — Session vRPC dispatch + slot lifecycle - This PR — Session lifecycle
Six nits from the igor-reviewer-agent review of PR googleapis#20211: - Introduce typed const SessionEventKind (close/hb-missed/hb-alive/ ctx-done/retry) so writers landing in the follow-up vRPC and lifecycle PRs can't drift on the string literal. - Replace the O(N) copy-shift in recordEvent with a wrap-index scheme matching latencySamples' existing pattern (eventsNext cursor). - peerInfoSummary: %x → %d for AfeID and gfe — the rest of the pkg renders these as decimal. - setChannelIndex: unexport (writer is package-internal) and take int32 directly instead of silently narrowing from int. - sessionDebug.init: take *sessionTracer instead of SessionType so the redundancy with Session.sessionType goes away. NewSession threads newSessionTracer(sessionType) through at the call site. - Add a doc comment on the sessionDebug struct pointing at the follow-up PRs where the counter / close-reason / remote-addr writers land.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 23, 2026
Introduces Session.Invoke and the one-in-flight vRPC dispatch that
routes server frames back to callers. Stacks on the sessionDebug bundle
(bigtable-session-debug); the counter/tracer/event writers land here.
New file: session_vrpc.go (~420 LOC)
- Session.Invoke — public entry point. Serializes claimSlot → Send →
awaitInvokeResult; deliver-vs-cancel branching lives in routeVRPCFrame.
- Slot lifecycle: claimSlot / drainSlot / markCancelled / activeVRPC —
slotMu-guarded, one vRPC in flight per session.
- handleVRPCResponse / handleVRPCErrorResponse + shared routeVRPCFrame —
Java-parity drain path with nil-active-VRPC + id-mismatch guards.
- deliver — writes to the caller's resultChan.
- cancelActiveRPCs — session-teardown path used by ForceClose /
handleGoAway / heartbeat watchdog.
- ForceClose — minimal port (transitionTo(Closed) → cancelActiveRPCs →
signalQuiescent). The full lifecycle-flavored ForceClose (setCloseReason
+ notifyClosing + notifyClosed + stream cleanup) lands with the
session_lifecycle follow-up.
Edits to session.go
- Add heartbeatWake chan struct{} field (cap-1 non-blocking) +
make(...) in NewSession — reactive watchdog wake channel.
- Add OnSlotDrained func() to SessionHooks + onSlotDrained() dispatcher.
Sole "session became free" signal; consumers wake pool waiters.
- Add Send(req) — mutex-guarded stream.Send wrapper; used by both this
PR's Invoke and the follow-up lifecycle Start.
- Add resetHeartbeatDeadline + wakeHeartbeatLoop — atomic-based reactive
wake pair; consumed by the follow-up lifecycle heartBeatLoop.
Edits to session_test.go
- Test fixtures: fakeStream / fakeDesc / newFakeStream / newRoundTripDesc /
makeActive / waitFor. Consumed by both this PR's TestInvoke*/TestHandleVRPC*
and the follow-up lifecycle tests.
- newTestSession signature change (stream, hooks); pre-existing 1-arg
call sites now use newDefaultTestSession shortcut.
Edits to debug_tracer.go
- tagSessionVRPCCancelledDrained — bookkeeping tag when a server response
drains a slot whose caller already ctx.Done'd.
Stack:
- Prereq: PR googleapis#20211 (bigtable-session-debug) — Session debug surface.
- Follow-up: session_lifecycle.go (Start, Close, readLoop, heartBeatLoop,
handleSessionResponse / handleOpenSession / handleGoAway / handleClose /
handleErrorResponse / peerInfoExtracter). Will extend ForceClose to its
full lifecycle-shaped body.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 23, 2026
…Loop, heartBeatLoop) 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 bigtable-session-vrpc. Stacks on bigtable-session-vrpc; deletes the minimal ForceClose stub that PR shipped, replacing it with the full lifecycle-shaped body. 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 update the deadline via resetHeartbeatDeadline; the unknown-payload branch records a debug tag but doesn't reset the deadline (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 - Add hookCounts helper (start/active/close callback counters). - Add setSlotForTest helper (seed the in-flight slot from tests). Stack: - googleapis#20211 — Session debug surface - googleapis#20213 — Session vRPC dispatch + slot lifecycle - This PR — Session lifecycle
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 24, 2026
…Loop, heartBeatLoop) 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 bigtable-session-vrpc. Stacks on bigtable-session-vrpc; deletes the minimal ForceClose stub that PR shipped, replacing it with the full lifecycle-shaped body. 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 update the deadline via resetHeartbeatDeadline; the unknown-payload branch records a debug tag but doesn't reset the deadline (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 - Add hookCounts helper (start/active/close callback counters). - Add setSlotForTest helper (seed the in-flight slot from tests). Stack: - googleapis#20211 — Session debug surface - googleapis#20213 — Session vRPC dispatch + slot lifecycle - This PR — Session lifecycle
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 24, 2026
Applies the same nits accepted on PRs googleapis#20211/googleapis#20213/googleapis#20215 to the sessionz working branch. See per-PR commits cf200ae / c51caae / 1f8a958 for rationale. Plus the ForceClose(nil) spurious-tag fix from 35d3b16.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 24, 2026
…Loop, heartBeatLoop) 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 bigtable-session-vrpc. Stacks on bigtable-session-vrpc; deletes the minimal ForceClose stub that PR shipped, replacing it with the full lifecycle-shaped body. 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 update the deadline via resetHeartbeatDeadline; the unknown-payload branch records a debug tag but doesn't reset the deadline (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 - Add hookCounts helper (start/active/close callback counters). - Add setSlotForTest helper (seed the in-flight slot from tests). Stack: - googleapis#20211 — Session debug surface - googleapis#20213 — Session vRPC dispatch + slot lifecycle - This PR — Session lifecycle
Contributor
Author
|
@igor-reviewer-agent thanks for the pass — addressed the 6 code nits in
Left the wide-surface concern as-is — those accessors are read by sessionz/debugview in the follow-up PRs; unexporting them would just require re-exporting later. Doc comment now makes the follow-up path explicit. |
mutianf
approved these changes
Jul 24, 2026
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 24, 2026
Introduces Session.Invoke and the one-in-flight vRPC dispatch that
routes server frames back to callers. Stacks on the sessionDebug bundle
(bigtable-session-debug); the counter/tracer/event writers land here.
New file: session_vrpc.go (~420 LOC)
- Session.Invoke — public entry point. Serializes claimSlot → Send →
awaitInvokeResult; deliver-vs-cancel branching lives in routeVRPCFrame.
- Slot lifecycle: claimSlot / drainSlot / markCancelled / activeVRPC —
slotMu-guarded, one vRPC in flight per session.
- handleVRPCResponse / handleVRPCErrorResponse + shared routeVRPCFrame —
Java-parity drain path with nil-active-VRPC + id-mismatch guards.
- deliver — writes to the caller's resultChan.
- cancelActiveRPCs — session-teardown path used by ForceClose /
handleGoAway / heartbeat watchdog.
- ForceClose — minimal port (transitionTo(Closed) → cancelActiveRPCs →
signalQuiescent). The full lifecycle-flavored ForceClose (setCloseReason
+ notifyClosing + notifyClosed + stream cleanup) lands with the
session_lifecycle follow-up.
Edits to session.go
- Add heartbeatWake chan struct{} field (cap-1 non-blocking) +
make(...) in NewSession — reactive watchdog wake channel.
- Add OnSlotDrained func() to SessionHooks + onSlotDrained() dispatcher.
Sole "session became free" signal; consumers wake pool waiters.
- Add Send(req) — mutex-guarded stream.Send wrapper; used by both this
PR's Invoke and the follow-up lifecycle Start.
- Add resetHeartbeatDeadline + wakeHeartbeatLoop — atomic-based reactive
wake pair; consumed by the follow-up lifecycle heartBeatLoop.
Edits to session_test.go
- Test fixtures: fakeStream / fakeDesc / newFakeStream / newRoundTripDesc /
makeActive / waitFor. Consumed by both this PR's TestInvoke*/TestHandleVRPC*
and the follow-up lifecycle tests.
- newTestSession signature change (stream, hooks); pre-existing 1-arg
call sites now use newDefaultTestSession shortcut.
Edits to debug_tracer.go
- tagSessionVRPCCancelledDrained — bookkeeping tag when a server response
drains a slot whose caller already ctx.Done'd.
Stack:
- Prereq: PR googleapis#20211 (bigtable-session-debug) — Session debug surface.
- Follow-up: session_lifecycle.go (Start, Close, readLoop, heartBeatLoop,
handleSessionResponse / handleOpenSession / handleGoAway / handleClose /
handleErrorResponse / peerInfoExtracter). Will extend ForceClose to its
full lifecycle-shaped body.
sushanb
added a commit
to sushanb/google-cloud-go
that referenced
this pull request
Jul 27, 2026
…Loop, heartBeatLoop) 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 bigtable-session-vrpc. Stacks on bigtable-session-vrpc; deletes the minimal ForceClose stub that PR shipped, replacing it with the full lifecycle-shaped body. 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 update the deadline via resetHeartbeatDeadline; the unknown-payload branch records a debug tag but doesn't reset the deadline (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 - Add hookCounts helper (start/active/close callback counters). - Add setSlotForTest helper (seed the in-flight slot from tests). Stack: - googleapis#20211 — Session debug surface - googleapis#20213 — Session vRPC dispatch + slot lifecycle - This PR — Session lifecycle
sushanb
added a commit
that referenced
this pull request
Jul 27, 2026
…Loop, 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 tests
3 tasks
sushanb
added a commit
that referenced
this pull request
Jul 27, 2026
…#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.
sushanb
pushed a commit
that referenced
this pull request
Aug 3, 2026
🤖 I have created a release *beep* *boop* --- ## [1.52.0](bigtable/v1.51.0...bigtable/v1.52.0) (2026-08-03) ### Features * **bigtable:** Add AFE picker (Simple / LeastInFlight / LeastLatency) ([#20204](#20204)) ([bcbf714](bcbf714)) * **bigtable:** Add ClientConfig.DisableSession to opt out of session backend ([#20297](#20297)) ([7ee5e44](7ee5e44)) * **bigtable:** Add getClientConfigDirectAccessChecker for session pools ([#20209](#20209)) ([3b8d30a](3b8d30a)) * **bigtable:** Add NoOpChannelPrimer for session channel pools ([#20208](#20208)) ([d055a8a](d055a8a)) * **bigtable:** Add per-AFE sessionList for the two-tier session pool ([#20224](#20224)) ([dbf0c3f](dbf0c3f)) * **bigtable:** Add protoRowToRow conversion helper for TableShim ([#20257](#20257)) ([1297143](1297143)) * **bigtable:** Add Session debug surface (observability fields + methods) ([#20211](#20211)) ([d8d3e16](d8d3e16)) * **bigtable:** Add Session lifecycle (Start, Close, ForceClose, readLoop, heartBeatLoop) ([#20215](#20215)) ([b9e53c6](b9e53c6)) * **bigtable:** Add Session struct + state machine ([#20117](#20117)) ([09acbb3](09acbb3)) * **bigtable:** Add session.Config.EnableDebug to gate sessionz debug state ([#20247](#20247)) ([ce74c31](ce74c31)) * **bigtable:** Add SessionClient + SessionTable + lazyPool ([#20228](#20228)) ([ab2c96c](ab2c96c)) * **bigtable:** Add SessionPoolImpl (two-tier pool + scaling + debug) ([#20225](#20225)) ([683eda8](683eda8)) * **bigtable:** Rename session pool display to <resource-id>-<PERM> ([#20248](#20248)) ([35e146e](35e146e)) * **bigtable:** Route Client.Open()-returned *Table through the Diverter ([#20273](#20273)) ([2b81c7d](2b81c7d)) * **bigtable:** State-based classification for abnormal session close ([#20243](#20243)) ([f2905b7](f2905b7)) * **bigtable:** TableShim fallback to classic on session UNIMPLEMENTED ([#20269](#20269)) ([36540af](36540af)) * **bigtable:** TTL-on-idle cache for per-resource session.TableAPI ([#20263](#20263)) ([00b2a49](00b2a49)) * **bigtable:** Wire Diverter on Client and route Open* via TableShim ([#20256](#20256)) ([b32fbd7](b32fbd7)) ### Bug Fixes * **bigtable:** AFE picker latency signal — subtract poolWait and compute TransportLatency = wire − backend at source ([#20281](#20281)) ([bb8c4d5](bb8c4d5)) * **bigtable:** Guard NewStream OnFinish against grpc-go double-fire ([#20295](#20295)) ([b51da29](b51da29)) * **bigtable:** Real per-resource pool teardown on sessionTable.Close + cache close-race gate ([#20264](#20264)) ([599aea9](599aea9)) * **bigtable:** Session.durations / session.uptime — set explicit histogram bucket boundaries ([#20276](#20276)) ([97eee22](97eee22)) * **bigtable:** SessionTableHandle self-heals across cache eviction ([#20296](#20296)) ([0dd98cd](0dd98cd)) * **bigtable:** Translate ctx errors to gRPC status on session vRPC ([#20299](#20299)) ([0f3b2a5](0f3b2a5)) * **bigtable:** Treat PingAndWarm NotFound as a successful prime ([#20219](#20219)) ([a1557ad](a1557ad)) ### Performance Improvements * **bigtable:** Delete periodic Tick loop; sizing is event-driven ([#20285](#20285)) ([2c096bd](2c096bd)) * **bigtable:** Drop pick_lost_race debug tag from CheckoutSession hot path ([#20280](#20280)) ([bd0e400](bd0e400)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)
Edits to `session.go`:
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
Test plan