feat: add support for configuring minimum num gRPC channels on CacheClient #345
Conversation
…lient Prior to this commit, each instance of the CacheClient would always open a single gRPC channel to the server. This commit adds a configuration setting on the GrpcConfiguration object which allows a user to configure a minimum number of channels; this is useful for cases where callers will be issuing more than 100 concurrent requests, since 100 is the maximum number of requests that can be multiplexed over a single channel.
| @@ -48,6 +54,8 @@ final class ScsDataGrpcStubsManager implements AutoCloseable { | |||
| ScsDataGrpcStubsManager( | |||
| @Nonnull CredentialProvider credentialProvider, @Nonnull Configuration configuration) { | |||
| this.deadline = configuration.getTransportStrategy().getGrpcConfiguration().getDeadline(); | |||
| this.numGrpcChannels = | |||
| configuration.getTransportStrategy().getGrpcConfiguration().getMinNumGrpcChannels(); | |||
There was a problem hiding this comment.
just wondering, why name it minNumGrpcChannels when it looks like its the exact number of grpc channels that get created. min to me implies that we will create more over time if we need to
There was a problem hiding this comment.
unless that is a future optimization that we plan on doing
There was a problem hiding this comment.
astute observation!
You guessed exactly right. In an ideal world I would like to make this dynamic; keep track of how many open requests they have, and automatically add more channels as needed. But that is a bigger/riskier change that will require a lot of perf testing, so I don't want to tackle it right now.
In the future, if we add that, then this setting would become the floor rather than the exact number of channels.
lmk if that makes sense.
Prior to this commit, each instance of the CacheClient would always
open a single gRPC channel to the server. This commit adds a configuration
setting on the GrpcConfiguration object which allows a user to configure
a minimum number of channels; this is useful for cases where callers will
be issuing more than 100 concurrent requests, since 100 is the maximum
number of requests that can be multiplexed over a single channel.