feat(bigtable): add lazyPool helper for on-demand session pool opening - #20182
Merged
Merged
Conversation
Adds the internal `session` package with a `lazyPool` primitive that opens its underlying Invoker on first use. Concurrent callers block until the open completes; failed opens are NOT cached so a transient proto.Marshal failure cannot strand the caller for the process lifetime. A nil *lazyPool or one with a nil `open` closure returns (nil, nil) so callers can model "no session support, use fallback" (e.g., the write side of a read-only materialized view). Also introduces `transport.InvokeResult` — the value type returned by `Session.Invoke` — so the session package can declare the `Invoker` interface without depending on the full session pool implementation that will land in a follow-up. Tests cover the nil-receiver / nil-open contract and the failed-open-not-cached invariant (with the counterfactual counter).
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a lazyPool mechanism to lazily initialize session pools on first use, along with a struct InvokeResult to capture RPC outputs. The review feedback highlights a package name inconsistency in invoke_result.go (using internal instead of matching the directory or other files). Additionally, the reviewer recommends refactoring lazyPool to avoid holding an exclusive mutex during the slow, synchronous open() operation, suggesting a coordination channel instead to prevent blocking concurrent status checks.
Summary
bigtable/internal/sessionwith alazyPoolprimitive that opens its underlyingInvokeron first use. Concurrent callers block until the open completes; failed opens are NOT cached, so a transientproto.Marshalfailure cannot strand the caller for the process lifetime.*lazyPoolor one with a nilopenclosure returns(nil, nil), letting callers model "no session support, use fallback" (e.g., the write side of a read-only materialized view).transport.InvokeResult— the value type returned bySession.Invoke— so the session package can declare theInvokerinterface without pulling in the full session-pool implementation, which will land in a follow-up.Test plan
go build ./bigtable/internal/session/... ./bigtable/internal/transport/...go vet ./bigtable/internal/session/... ./bigtable/internal/transport/...go test ./bigtable/internal/session/... -run LazyPool -count=1(bothTestLazyPool_NilPoolAndNilOpenReturnNilNilandTestLazyPool_FailedOpenNotCachedpass)go test ./bigtable/internal/session/... -count=1 -race