Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request exposes the gRPC channel pool configuration publicly for the Spanner client, allowing users to configure either a static or a dynamic load-based channel pool. It transitions internal channel pool configuration types and strategies to public APIs, adds the with_channel_pool builder method, supports configuring the dynamic channel pool via the SPANNER_ENABLE_DYNAMIC_CHANNEL_POOL environment variable, and integrates session priming with the channel pool. Additionally, it cleans up unused code, refactors tests, and updates documentation. I have no feedback to provide.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6834 +/- ##
==========================================
- Coverage 96.92% 96.02% -0.91%
==========================================
Files 327 328 +1
Lines 108648 110756 +2108
==========================================
+ Hits 105309 106353 +1044
- Misses 3339 4403 +1064 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
6fe035b to
682110c
Compare
| } | ||
| } | ||
|
|
||
| /// Returns the provided affinity handle, or creates a new default `ReadOnly` affinity if `None`. |
There was a problem hiding this comment.
Removed, as they were not in use
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
Removed, as the methods that it tested were removed.
| Self { guard } | ||
| } | ||
|
|
||
| /// Consumes the lease, returning the underlying active RPC guard. |
There was a problem hiding this comment.
The methods that are being removed here are also removed because they were not used. (Note that error penalties are being used by this implementation, it was just the method here that is not needed)
| let session = | ||
| Self::create_session(&spanner, &database_name, &database_role, &options, &o11y).await?; | ||
|
|
||
| spanner |
There was a problem hiding this comment.
This is strictly speaking a bug fix that came up while working on this, and not directly related to making it publicly available. I did not create a separate PR for it, as the pool was not yet public before this, meaning that no one has been able to use the pool without this priming.
|
|
||
| ### Configuring the Channel Pool | ||
|
|
||
| By default, the client uses a static pool of 4 gRPC channels. You can configure |
There was a problem hiding this comment.
The default is likely to be changed to dynamic in the (near) future, once we have benchmarked it.
682110c to
c1cb749
Compare
| pub fn initial_channels(&self) -> usize { | ||
| self.initial_channels | ||
| } | ||
|
|
||
| /// Returns the minimum number of channels retained during scale-down. | ||
| pub fn min_channels(&self) -> usize { | ||
| self.min_channels | ||
| } | ||
|
|
||
| /// Returns the maximum number of channels allowed during scale-up. | ||
| pub fn max_channels(&self) -> usize { | ||
| self.max_channels | ||
| } | ||
|
|
||
| /// Returns the low-load threshold (per channel) triggering scale-down evaluation. | ||
| pub fn min_rpc_per_channel(&self) -> f64 { | ||
| self.min_rpc_per_channel | ||
| } | ||
|
|
||
| /// Returns the high-load threshold (per channel) triggering scale-up. | ||
| pub fn max_rpc_per_channel(&self) -> f64 { | ||
| self.max_rpc_per_channel | ||
| } | ||
|
|
||
| /// Returns the synthetic picker load added per qualifying transport error. | ||
| pub fn error_penalty_step(&self) -> u32 { | ||
| self.error_penalty_step | ||
| } | ||
|
|
||
| /// Returns the sliding window duration for active error penalties. | ||
| pub fn error_penalty_duration(&self) -> Duration { | ||
| self.error_penalty_duration | ||
| } | ||
|
|
||
| /// Returns the interval between periodic scale-down evaluations. | ||
| pub fn scale_down_check_interval(&self) -> Duration { | ||
| self.scale_down_check_interval | ||
| } | ||
|
|
||
| /// Returns the cooldown period between consecutive scale-up bursts. | ||
| pub fn scale_up_cooldown(&self) -> Duration { | ||
| self.scale_up_cooldown | ||
| } | ||
|
|
||
| /// Returns the number of consecutive low-load checks required before scale-down. | ||
| pub fn consecutive_low_load_checks(&self) -> usize { | ||
| self.consecutive_low_load_checks | ||
| } | ||
|
|
||
| /// Returns the maximum percentage of current pool size added per scale-up event. | ||
| pub fn max_scale_up_percent(&self) -> u32 { | ||
| self.max_scale_up_percent | ||
| } | ||
|
|
||
| /// Returns the maximum number of channels marked draining per scale-down cycle. | ||
| pub fn max_remove_channels(&self) -> usize { | ||
| self.max_remove_channels | ||
| } | ||
|
|
||
| /// Returns the idle grace duration a draining channel is kept alive after load drops to zero. | ||
| pub fn drain_idle_grace(&self) -> Duration { | ||
| self.drain_idle_grace | ||
| } | ||
|
|
||
| /// Returns the timeout for executing `SELECT 1` priming on a new scaled-up channel. | ||
| pub fn prime_timeout(&self) -> Duration { | ||
| self.prime_timeout | ||
| } | ||
|
|
||
| /// Returns the maximum retry attempts for `SELECT 1` priming. | ||
| pub fn prime_max_attempts(&self) -> usize { | ||
| self.prime_max_attempts | ||
| } | ||
|
|
||
| /// Returns the channel selection strategy. | ||
| pub fn selection_strategy(&self) -> ChannelSelectionStrategy { | ||
| self.selection_strategy | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: do we really need to make all those available to read ? At this point, is almost just easier to make the fields in ChannelSelectionStrategy pub. I would remove those methods and let customers request it (unless there is a known need).
Just to keep in mind that making something public later is easier, but the opposite is a breaking change.
6828f84 to
ee909a9
Compare
|
(Reverted to Draft for now, as we are still looking into some shortcomings of this implementation) |
b97b1dc to
bab23df
Compare
Adds support for configuring gRPC channel pooling when creating a Spanner client. Customers can now configure the channel pool on `Spanner::builder()` using `.with_channel_pool(...)`: - Static channel pool (`StaticChannelPoolConfig`): Configures a fixed number of gRPC channels (defaults to 4 channels, or 1 channel when targeting the Spanner emulator). - Dynamic channel pool (`DynamicChannelPoolConfig`): Automatically scales the number of channels up or down based on RPC concurrency and load.
bab23df to
7cb1b28
Compare
2 participants
Adds support for configuring gRPC channel pooling when creating a Spanner client.
Customers can now configure the channel pool on
Spanner::builder()using.with_channel_pool(...):StaticChannelPoolConfig): Configures a fixed number of gRPC channels (defaults to 4 channels, or 1 channel when targeting the Spanner emulator).DynamicChannelPoolConfig): Automatically scales the number of channels up or down based on RPC concurrency and load.