perf(bigtable): drop pick_lost_race debug tag from CheckoutSession hot path - #20280
Merged
sushanb merged 2 commits intoJul 31, 2026
Merged
Conversation
…t path recordDebugTag on the pick-lost-race path (session_pool.go:286) fires per Checkout retry under contention. Each call takes a global RWMutex.RLock, does a sync.Map lookup, and allocates an OTel WithAttributes option — the design comment at debug_tracer.go:15-20 declares recordDebugTag "cheap enough to sprinkle on cold paths", and this was the sole hot-path violator. Sandbox smoke against sushanb-uc1 saw 5666 emissions in 24 minutes (~4/sec) with CBT_FORCE_SESSION=true — 100% waste since operators have no action to take from the count. Remove the recordDebugTag call and the accompanying tagSessionPoolPickLostRace constant declaration. Behavior at the pick site is unchanged — CheckoutSession still falls through to the park path via the existing loop; the counter was purely observability. Rationale for delete-not-gate: gating on p.debugEnabled adds a conditional to every retry for observability that nobody consumes except in synthetic loads. If a future need arises, the same signal is derivable from `sessionz` pick-decision history + concurrent in-use snapshots without a per-call counter.
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the tagSessionPoolPickLostRace debug tag and its tracking inside CheckoutSession to avoid performance overhead from global locks and allocations on a hot path during saturation. There are no review comments to evaluate, so no further feedback is provided.
Follow-up to the recordDebugTag removal — the leftover comment described a bookkeeping call that no longer exists. The code above speaks for itself: Checkout returned nil, the loop re-picks.
mutianf
approved these changes
Jul 31, 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
`recordDebugTag` on the pick-lost-race path (`session_pool.go:286`) fires per Checkout retry under contention. Each call takes a global `RWMutex.RLock`, does a `sync.Map` lookup, and allocates an OTel `WithAttributes` option — the design comment at `debug_tracer.go:15-20` declares `recordDebugTag` "cheap enough to sprinkle on cold paths", and this was the sole hot-path violator.
Measured impact
Sandbox smoke against `sushanb-uc1` saw 5666 emissions in 24 min (~4/sec) with `CBT_FORCE_SESSION=true`. Each fire is:
Multiplied across many concurrent `CheckoutSession` callers (each racing on the same handful of just-freed sessions), it's real steady-state overhead for observability nobody consumes.
Fix
Remove the `recordDebugTag` call and the accompanying `tagSessionPoolPickLostRace` constant declaration. Pick-lost-race behavior at the call site is unchanged — `CheckoutSession` still falls through to the park path via the existing loop; the counter was purely observability.
Rationale for delete-not-gate
Gating on `p.debugEnabled` adds a conditional per retry for observability that nobody consumes except in synthetic loads. If future analysis needs the signal, it's derivable from `sessionz` pick-decision history + concurrent in-use snapshots without a per-call counter.
Test plan