feat(spanner): add dynamic channel pool support#14604
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a Dynamic Channel Pool (DCP) for the Spanner client, which automatically scales the number of gRPC channels based on RPC load. The implementation includes features like Power-of-Two-Least-Busy selection, channel priming, graceful draining, and OpenTelemetry metrics. Review feedback highlights the need to improve configuration defaulting for initial channels and to propagate request contexts through the session management layer to ensure consistent observability and trace propagation.
| DCPMaxRemoveChannels: 2, | ||
| DCPDrainIdleGrace: 5 * time.Minute, | ||
| DCPMaxDrainTimeout: 30 * time.Minute, |
There was a problem hiding this comment.
The current defaulting logic for DCPInitialChannels can lead to validation failures if a user provides a DCPMinChannels value greater than the default initial channels (4). Consider defaulting DCPInitialChannels to at least DCPMinChannels to ensure consistency, while ensuring that this change does not break backward compatibility for existing stable code paths.
| DCPMaxRemoveChannels: 2, | |
| DCPDrainIdleGrace: 5 * time.Minute, | |
| DCPMaxDrainTimeout: 30 * time.Minute, | |
| if cfg.DCPInitialChannels == 0 { | |
| cfg.DCPInitialChannels = def.DCPInitialChannels | |
| if cfg.DCPInitialChannels < cfg.DCPMinChannels { | |
| cfg.DCPInitialChannels = cfg.DCPMinChannels | |
| } | |
| } |
References
- When introducing new components with different default behaviors, avoid altering the defaults of existing, stable code paths to prevent breaking changes for backward compatibility.
Split of #14604 Internal reference: go/go-dcp-design
Add Go Spanner Dynamic Channel Pooling (DCP) with scale-up/down, drain safety, DirectPath fallback support,
observability.
Internal reference: go/go-dcp-design