From 5916f730af113195e94f0c7f8f080a08da1c697f Mon Sep 17 00:00:00 2001 From: Chris Price Date: Thu, 22 Feb 2024 22:12:16 -0800 Subject: [PATCH] feat: async factory function for eager connections This commit makes the following changes: * Stop trying to do the eager connection stuff in the constructor of the CacheClient, because this can have surprising side effects, e.g. if we want to throw an error on failure. * Remove the associated Configuration settings since we no longer support eager connections at construction time. * Add a new factory function, CacheClient.CreateAsync, which is the new mechanism for create a client with an eager connection. This matches the pattern we established in the other SDKs that support eager connections. * If the eager connection fails, we now throw a new ConnectionError rather than just logging a warning. This gives the consumer the ability to decide how to handle this type of error rather than us just swallowing it and removing their choice. In most cases, users would just end up hitting a timeout error on their next request after we gave up on the eager connection. In the future we may add more configuration options for how to handle eager connection failures, possibly including some automatic retries. For now, this gives the user more control, which seems extremely desirable given the number of times we have recently seen users run into DNS throttling when they try to make a very high volume of connections to Momento from lambdas. --- src/Momento.Sdk/CacheClient.cs | 19 +++++++++++ src/Momento.Sdk/Config/Configurations.cs | 6 +--- .../Config/Transport/ITransportStrategy.cs | 19 ----------- .../Exceptions/ConnectionException.cs | 15 ++++++++ src/Momento.Sdk/Exceptions/SdkException.cs | 4 +++ src/Momento.Sdk/Internal/DataGrpcManager.cs | 34 ++++++++++--------- src/Momento.Sdk/Internal/ScsDataClient.cs | 5 +++ .../CacheEagerConnectionTest.cs | 16 ++------- tests/Unit/Momento.Sdk.Tests/ConfigTest.cs | 7 ---- 9 files changed, 65 insertions(+), 60 deletions(-) create mode 100644 src/Momento.Sdk/Exceptions/ConnectionException.cs diff --git a/src/Momento.Sdk/CacheClient.cs b/src/Momento.Sdk/CacheClient.cs index 0e0bf4b1..0d718142 100644 --- a/src/Momento.Sdk/CacheClient.cs +++ b/src/Momento.Sdk/CacheClient.cs @@ -35,6 +35,25 @@ private ScsDataClient DataClient protected readonly IConfiguration config; /// protected readonly ILogger _logger; + + /// + /// Async factory function to construct a Momento CacheClient with an eager connection to the + /// Momento server. Calling the CacheClient constructor directly will not establish a connection + /// immediately, but instead establish it lazily when the first request is issued. This factory + /// function will ensure that the connection is established before the first request. + /// + /// Configuration to use for the transport, retries, middlewares. See for out-of-the-box configuration choices, eg + /// Momento auth provider. + /// Default time to live for the item in cache. + /// Maximum time to wait for eager connection. + /// is zero or negative. + /// The eager connection could not be established within the specified + public static async Task CreateAsync(IConfiguration config, ICredentialProvider authProvider, TimeSpan defaultTtl, TimeSpan eagerConnectionTimeout) + { + CacheClient cacheClient = new CacheClient(config, authProvider, defaultTtl); + await cacheClient.DataClient.EagerConnectAsync(eagerConnectionTimeout); + return cacheClient; + } /// diff --git a/src/Momento.Sdk/Config/Configurations.cs b/src/Momento.Sdk/Config/Configurations.cs index 2d7e8a08..dc1f640e 100644 --- a/src/Momento.Sdk/Config/Configurations.cs +++ b/src/Momento.Sdk/Config/Configurations.cs @@ -184,11 +184,7 @@ private Lambda(ILoggerFactory loggerFactory, IRetryStrategy retryStrategy, ITran /// public static IConfiguration Latest(ILoggerFactory? loggerFactory = null) { - var config = Default.V1(loggerFactory); - var transportStrategy = config.TransportStrategy.WithEagerConnectionTimeout( - TimeSpan.FromSeconds(30) - ); - return config.WithTransportStrategy(transportStrategy); + return Default.V1(loggerFactory); } } } diff --git a/src/Momento.Sdk/Config/Transport/ITransportStrategy.cs b/src/Momento.Sdk/Config/Transport/ITransportStrategy.cs index 0d894552..78849ec5 100644 --- a/src/Momento.Sdk/Config/Transport/ITransportStrategy.cs +++ b/src/Momento.Sdk/Config/Transport/ITransportStrategy.cs @@ -14,14 +14,6 @@ public interface ITransportStrategy /// public int MaxConcurrentRequests { get; } - /// - /// If null, the client will only attempt to connect to the server lazily when the first request is executed. - /// If provided, the client will attempt to connect to the server immediately upon construction; if the connection - /// cannot be established within the specified TimeSpan, it will abort the connection attempt, log a warning, - /// and proceed with execution so that the application doesn't hang. - /// - public TimeSpan? EagerConnectionTimeout { get; } - /// /// Configures the low-level gRPC settings for the Momento client's communication /// with the Momento server. @@ -50,15 +42,4 @@ public interface ITransportStrategy /// /// A new ITransportStrategy with the specified client timeout public ITransportStrategy WithClientTimeout(TimeSpan clientTimeout); - - /// - /// Copy constructor to enable eager connection to the server - /// - /// A timeout for attempting an eager connection to the server. When the client - /// is constructed, it will attempt to connect to the server immediately. If the connection - /// cannot be established within the specified TimeSpan, it will abort the connection attempt, log a warning, - /// and proceed with execution so that the application doesn't hang. - /// - /// A new ITransportStrategy configured to eagerly connect to the server upon construction - public ITransportStrategy WithEagerConnectionTimeout(TimeSpan connectionTimeout); } diff --git a/src/Momento.Sdk/Exceptions/ConnectionException.cs b/src/Momento.Sdk/Exceptions/ConnectionException.cs new file mode 100644 index 00000000..87ced02e --- /dev/null +++ b/src/Momento.Sdk/Exceptions/ConnectionException.cs @@ -0,0 +1,15 @@ +namespace Momento.Sdk.Exceptions; + +using System; + +/// +/// Unable to connect to the server. +/// +public class ConnectionException : SdkException +{ + /// + public ConnectionException(string message, MomentoErrorTransportDetails transportDetails, Exception? e = null) : base(MomentoErrorCode.CONNECTION_ERROR, message, transportDetails, e) + { + this.MessageWrapper = "Unable to connect to the server; consider retrying. If the error persists, please contact us at support@momentohq.com"; + } +} diff --git a/src/Momento.Sdk/Exceptions/SdkException.cs b/src/Momento.Sdk/Exceptions/SdkException.cs index cf3c5ab6..2822fca8 100644 --- a/src/Momento.Sdk/Exceptions/SdkException.cs +++ b/src/Momento.Sdk/Exceptions/SdkException.cs @@ -66,6 +66,10 @@ public enum MomentoErrorCode /// FAILED_PRECONDITION_ERROR, /// + /// Unable to connect to the server + /// + CONNECTION_ERROR, + /// /// Unknown error has occurred /// UNKNOWN_ERROR diff --git a/src/Momento.Sdk/Internal/DataGrpcManager.cs b/src/Momento.Sdk/Internal/DataGrpcManager.cs index b1210b1f..06d3eeff 100644 --- a/src/Momento.Sdk/Internal/DataGrpcManager.cs +++ b/src/Momento.Sdk/Internal/DataGrpcManager.cs @@ -15,6 +15,7 @@ using Momento.Sdk.Config; using Momento.Sdk.Config.Middleware; using Momento.Sdk.Config.Retry; +using Momento.Sdk.Exceptions; using Momento.Sdk.Internal.Middleware; using static System.Reflection.Assembly; @@ -299,24 +300,25 @@ internal DataGrpcManager(IConfiguration config, string authToken, string endpoin ).ToList(); var client = new Scs.ScsClient(invoker); - - if (config.TransportStrategy.EagerConnectionTimeout != null) + Client = new DataClientWithMiddleware(client, middlewares); + } + + internal async Task EagerConnectAsync(TimeSpan eagerConnectionTimeout) + { + _logger.LogDebug("Attempting eager connection to server"); + var pingClient = new Ping.PingClient(this.channel); + try { - TimeSpan eagerConnectionTimeout = config.TransportStrategy.EagerConnectionTimeout.Value; - _logger.LogDebug("TransportStrategy EagerConnection is enabled; attempting to connect to server"); - var pingClient = new Ping.PingClient(this.channel); - try - { - pingClient.Ping(new _PingRequest(), - new CallOptions(deadline: DateTime.UtcNow.Add(eagerConnectionTimeout))); - } - catch (RpcException ex) - { - _logger.LogWarning($"Failed to eagerly connect to the server; continuing with execution in case failure is recoverable later: {ex}"); - } + await pingClient.PingAsync(new _PingRequest(), + new CallOptions(deadline: DateTime.UtcNow.Add(eagerConnectionTimeout))); + } + catch (RpcException ex) + { + MomentoErrorTransportDetails transportDetails = new MomentoErrorTransportDetails( + new MomentoGrpcErrorDetails(ex.StatusCode, ex.Message, null) + ); + throw new ConnectionException("Eager connection to server failed", transportDetails, ex); } - - Client = new DataClientWithMiddleware(client, middlewares); } public void Dispose() diff --git a/src/Momento.Sdk/Internal/ScsDataClient.cs b/src/Momento.Sdk/Internal/ScsDataClient.cs index 3750f1ab..024b1271 100644 --- a/src/Momento.Sdk/Internal/ScsDataClient.cs +++ b/src/Momento.Sdk/Internal/ScsDataClient.cs @@ -33,6 +33,11 @@ public ScsDataClientBase(IConfiguration config, string authToken, string endpoin this._logger = config.LoggerFactory.CreateLogger(); this._exceptionMapper = new CacheExceptionMapper(config.LoggerFactory); } + + internal Task EagerConnectAsync(TimeSpan eagerConnectionTimeout) + { + return this.grpcManager.EagerConnectAsync(eagerConnectionTimeout); + } protected Metadata MetadataWithCache(string cacheName) { diff --git a/tests/Integration/Momento.Sdk.Tests/CacheEagerConnectionTest.cs b/tests/Integration/Momento.Sdk.Tests/CacheEagerConnectionTest.cs index c8e3f286..6b1541e4 100644 --- a/tests/Integration/Momento.Sdk.Tests/CacheEagerConnectionTest.cs +++ b/tests/Integration/Momento.Sdk.Tests/CacheEagerConnectionTest.cs @@ -29,15 +29,6 @@ public void CacheClientConstructor_LazyConnection() var client = new CacheClient(config, authProvider, defaultTtl); } - [Fact] - public void CacheClientConstructor_EagerConnection_Success() - { - var config = Configurations.Laptop.Latest(loggerFactory); - config = config.WithTransportStrategy(config.TransportStrategy.WithEagerConnectionTimeout(TimeSpan.FromSeconds(5))); - // just validating that we can construct the client when the eager connection is successful - var client = new CacheClient(config, authProvider, defaultTtl); - } - [Fact] public void CacheClientConstructor_WithChannelsAndMaxConn_Success() { @@ -56,13 +47,12 @@ public void CacheClientConstructor_WithChannelsAndMaxConn_Success() } [Fact] - public void CacheClientConstructor_EagerConnection_BadEndpoint() + public async void CacheClientCreate_EagerConnection_BadEndpoint() { var config = Configurations.Laptop.Latest(loggerFactory); - config = config.WithTransportStrategy(config.TransportStrategy.WithEagerConnectionTimeout(TimeSpan.FromSeconds(2))); var authProviderWithBadCacheEndpoint = authProvider.WithCacheEndpoint("cache.cell-external-beta-1.prod.a.momentohq.com:65000"); Console.WriteLine($"Hello developer! We are about to run a test that verifies that the cache client is still operational even if our eager connection (ping) fails. So you will see the test log a warning message about that. It's expected, don't worry!"); - // validating that the constructor doesn't fail when the eager connection fails - var client = new CacheClient(config, authProviderWithBadCacheEndpoint, defaultTtl); + + await Assert.ThrowsAsync(async () => await CacheClient.CreateAsync(config, authProviderWithBadCacheEndpoint, defaultTtl, TimeSpan.FromSeconds(2))); } } diff --git a/tests/Unit/Momento.Sdk.Tests/ConfigTest.cs b/tests/Unit/Momento.Sdk.Tests/ConfigTest.cs index 9b86fecd..78e49fda 100644 --- a/tests/Unit/Momento.Sdk.Tests/ConfigTest.cs +++ b/tests/Unit/Momento.Sdk.Tests/ConfigTest.cs @@ -19,11 +19,4 @@ public void V1VConfigs_EqualLatest_HappyPath() Assert.Equal(Configurations.InRegion.Default.Latest(), Configurations.InRegion.Default.V1()); Assert.Equal(Configurations.InRegion.LowLatency.Latest(), Configurations.InRegion.LowLatency.V1()); } - - [Fact] - public void LambdaLatest_HasEagerConnectionTimeout_HappyPath() - { - var config = Configurations.InRegion.Lambda.Latest(); - Assert.Equal(TimeSpan.FromSeconds(30), config.TransportStrategy.EagerConnectionTimeout); - } }