feat(bigtable): state-based classification for abnormal session close - #20243
Merged
sushanb merged 3 commits intoJul 28, 2026
Merged
Conversation
…arity)
Move the consecutive-failure counter's abnormal-close gate from a
reason-string whitelist to the state history captured at the terminal
transitionTo(StateClosed) sites. Matches SessionPoolImpl.java's
`if (prevState != WAIT_SERVER_CLOSE) { consecutiveFailures++; ... }`.
Changes:
- Session gains prevStateAtClose atomic.Int32, stamped once at the two
transitionTo(StateClosed, ...) call sites (ForceClose + handleClose).
- SessionPoolImpl.noteAbnormalCloseIfAny now checks
`prevStateAtClose == StateWaitServerClose` — clean-close (Close() →
WSC → server ack → Closed) is exempt; every other terminal transition
(server GoAway, heartbeat trip, stream error on a Ready session, or
never-activated open failure) counts as abnormal. Also emits
tagSessionAbnormalClose on the counted path for per-abnormal-close
volume visibility in debug-tag counters.
- Delete isAbnormalCloseReason() and its call in handleClose — the
state-based classifier makes the reason-string whitelist redundant.
- Delete streamEndReason() and its handleClose call — it existed only
to feed the deleted classifier; upstream close paths still stamp
specific reasons (CAS-once semantics preserved).
- Drop 3 now-unused imports from session_lifecycle.go.
- Update test fixtures: abnormalOnCloseFor(false) and
TestConsecutiveFailures_UserReasonNotAbnormal stamp prevStateAtClose
= WSC before onClose to match real session.Close() → handleClose
state history.
Net -49 LOC; all internal/transport tests pass under -race.
Restores the sessionz closesByReason observability that the previous state-based-classification refactor dropped. Transport-level closes (readLoop.Recv returning a gRPC status error with no prior close-reason stamp) previously showed as "Unspecified" in sessionz's histogram; they now show as "StreamEnd:Unavailable", "StreamEnd:Canceled", etc. so operators can distinguish transport-drop causes. Kept minimal — one inline stamp guarded by err != nil. setCloseReason is CAS-once, so upstream stampers (GoAway, MissedHeartbeat, Error, User, etc.) still win when they ran first. Also strips Java-parity language from three doc comments touched by the recent state-based-classification refactor (per the Go docstring rule — describe Go behavior, not the reference implementation). Files: - session_lifecycle.go: +1 stamp line, +1 status import, -Java ref in handleSessionResponse docstring - session.go: -Java ref from prevStateAtClose docstring - session_pool_lifecycle.go: -2 Java refs from noteAbnormalCloseIfAny docblock + inline comment
status.Code(io.EOF) returns codes.Unknown, so the default arm of handleClose's close-reason stamper renders a graceful server-side shutdown as "StreamEnd:Unknown" — hiding a distinct, common signal in sessionz's "Unspecified" bucket. Special-case errors.Is(err, io.EOF) → "StreamEnd:EOF" before falling through to the default arm. Ctx errors (Canceled, DeadlineExceeded) are already mapped by grpc-go's status helpers, so the default arm still covers them correctly.
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the session closing classification from a reason-string-based mechanism to a state-history-based mechanism. It introduces prevStateAtClose in the Session struct to capture the state immediately before transitioning to StateClosed, allowing the session pool to accurately identify abnormal closes. The helper functions streamEndReason and isAbnormalCloseReason have been removed, and tests have been updated accordingly. There are no review comments, so I have no additional feedback to provide.
mutianf
approved these changes
Jul 28, 2026
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
Bring the Go client's abnormal session-close classification in line with Java by keying on the pre-close
State, and stamp aStreamEnd:<code>close-reason so sessionz surfaces exit codes that would otherwise land in the "Unspecified" bucket.refactor(bigtable): state-based abnormal-close classification (Java parity)—noteAbnormalCloseIfAnynow consultsSession.prevStateAtClose. Any session whose prior state was NOTStateWaitServerClosecounts as abnormal; sessions that gracefully enteredWAIT_SERVER_CLOSE(client-initiatedCloseSession→ server ACK) do not. Matches Java'sSessionImpl.handleAbnormalClosegating onprevState != WAIT_SERVER_CLOSE. Heartbeat-miss sessions (ForceClose fromReady→prev=Ready) correctly still count as abnormal.observe(bigtable): stamp StreamEnd:<code> close-reason in handleClose— the terminalhandleClosepath CAS-stamps aStreamEnd:<grpc-code>reason so a bare stream error no longer renders as "Unspecified" on sessionz. Upstream stampers (GoAway,MissedHeartbeat,Error,User) still win the CAS becausesetCloseReasonis one-shot — they run first and this stamp is a strict fallback.fix(bigtable): special-case io.EOF in StreamEnd close-reason— Igor caught during review:status.Code(io.EOF) == codes.Unknown, so the default arm rendered a graceful server-side shutdown as\"StreamEnd:Unknown\"— hiding a distinct-and-common signal. Special-caseserrors.Is(err, io.EOF)→\"StreamEnd:EOF\"before falling through.Test plan
go test ./bigtable/internal/transport/ -count=1 -race -short -timeout=120sclose_reasonrenders asGoAway(notStreamEnd:EOF), i.e. the upstream GoAway stamper still wins the CAS.close_reason = StreamEnd:EOF(notUnspecified, notStreamEnd:Unknown).MissedHeartbeatstill wins the CAS (this PR doesn't change that path, but verify no regression).Reviewer notes
Three reviewers ran clean:
session-reviewer(behavioral — 4 specs),session-component-review(boundaries — Part B + Part C),igor-reviewer(persona). No spec drift;setCloseReasonremains CAS-once and no upstream stampers are demoted.