Skip to content

Commit

Permalink
fix: fix configuration settings for maxConn and gRPC channels
Browse files Browse the repository at this point in the history
  • Loading branch information
pratik151192 committed Aug 2, 2023
1 parent ba0ef95 commit bb793b9
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 10 deletions.
34 changes: 30 additions & 4 deletions src/Momento.Sdk/CacheClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,38 @@ public CacheClient(IConfiguration config, ICredentialProvider authProvider, Time
this.dataClients = new List<ScsDataClient>();
int minNumGrpcChannels = this.config.TransportStrategy.GrpcConfig.MinNumGrpcChannels;
int currentMaxConcurrentRequests = this.config.TransportStrategy.MaxConcurrentRequests;
/**
* Client Configuration Logic:
*
* At the time of writing, customers have two client configurations affecting the number of gRPC connections spawned:
*
* 1. MinNumGrpcChannels: Determines the number of unique data clients to create based on the provided value.
* Each client eagerly creates one unique connection.
*
* 2. MaxConcurrentRequests: Configures each channel or data client to create a unique connection dynamically/lazily
* when 100 client concurrent requests are hit.
*
* For example, if we have 2 channels and a client provides a value of 200 for MaxConcurrentRequests, we can create a
* maximum of 2 * (200 / 100) ≈ 4 unique connections.
*
* Understanding Client Expectations:
*
* The client presumes that MaxConcurrentRequests is applied at a global level rather than per channel or data client.
* While some clients might utilize minNumGrpcChannels, it is expected that such clients are few and not the majority.
*
* Logic Implementation:
*
* This logic ensures that we honor the maxConcurrentRequests provided by a client if they also provide minNumGrpcChannels,
* and we internally "distribute" the max concurrent requests evenly over all the channels. This makes sure that
* we honor client's maxConcurrentRequests at a global level, and also create the number of channels they request.
* If they do not explicitly provide the number of channels, we default to 1 with the max concurrent requests
* applied to that single channel.
*/
if (minNumGrpcChannels > 1)
{

int newMaxConcurrentRequests = (currentMaxConcurrentRequests / minNumGrpcChannels);
ITransportStrategy transportStrategy = this.config.TransportStrategy;
transportStrategy = transportStrategy.WithMaxConcurrentRequests(newMaxConcurrentRequests);
int newMaxConcurrentRequests = Math.Max(1, currentMaxConcurrentRequests / minNumGrpcChannels);
ITransportStrategy transportStrategy = this.config.TransportStrategy
.WithMaxConcurrentRequests(newMaxConcurrentRequests);
this.config = this.config.WithTransportStrategy(transportStrategy);
_logger.LogWarning("Overriding maxConcurrentRequests for each gRPC channel to {}." +
" Min gRPC channels: {}, total maxConcurrentRequests: {}", newMaxConcurrentRequests,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ public void CacheClientConstructor_WithChannelsAndMaxConn_Success()
grpcConfiguration = grpcConfiguration.WithMinNumGrpcChannels(10);
config = config.WithTransportStrategy(config.TransportStrategy
.WithGrpcConfig(grpcConfiguration)
.WithMaxConcurrentRequests(500));
// still 500; clients shouldn't know we are doing 500/10 magic internally
Assert.Equal(500, config.TransportStrategy.MaxConcurrentRequests);
Assert.Equal(10, config.TransportStrategy.GrpcConfig.MinNumGrpcChannels);
.WithMaxConcurrentRequests(2));

// just validating that we can construct the client wh
var client = new CacheClient(config, authProvider, defaultTtl);
// still 2; clients shouldn't know we are doing 2/10 magic internally
Assert.Equal(2, config.TransportStrategy.MaxConcurrentRequests);
Assert.Equal(10, config.TransportStrategy.GrpcConfig.MinNumGrpcChannels);
}

[Fact]
Expand Down
2 changes: 0 additions & 2 deletions tests/Unit/Momento.Sdk.Tests/ConfigTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
using Momento.Sdk.Config;
using Momento.Sdk.Config.Transport;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;

public class ConfigTest
{
Expand Down

0 comments on commit bb793b9

Please sign in to comment.