From 7f73791a1cdfe02fd42ca6dc195c422dc4dd342c Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf <115584722+sfc-gh-ext-simba-lf@users.noreply.github.com> Date: Mon, 6 Nov 2023 10:06:28 -0800 Subject: [PATCH] SNOW-878067: Retry Strategy (#803) ### Description Regarding issue 575 The PR adds a new retry strategy for login requests with a new base time and adding jitter. It also adds the id and name to the header for login requests ### Checklist - [x] Code compiles correctly - [x] Code is formatted according to [Coding Conventions](../CodingConventions.md) - [x] Created tests which fail without the change (if possible) - [x] All tests passing (`dotnet test`) - [ ] Extended the README / documentation, if necessary - [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name --- README.md | 3 +- .../IntegrationTests/SFConnectionIT.cs | 240 ++++++++++++------ .../IntegrationTests/SFDbCommandIT.cs | 78 ++++++ .../Mock/MockRetryUntilRestTimeout.cs | 11 +- .../UnitTests/HttpUtilTest.cs | 44 ++++ .../UnitTests/SFSessionPropertyTest.cs | 10 +- .../Session/SFHttpClientPropertiesTest.cs | 126 +++++++-- Snowflake.Data/Core/HttpUtil.cs | 84 +++++- Snowflake.Data/Core/RestRequest.cs | 11 + Snowflake.Data/Core/Session/SFSession.cs | 6 +- .../Session/SFSessionHttpClientProperties.cs | 41 ++- .../Core/Session/SFSessionProperty.cs | 5 +- 12 files changed, 537 insertions(+), 122 deletions(-) diff --git a/README.md b/README.md index 440cd0e0a..13f2ca41d 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,8 @@ The following table lists all valid connection properties: | SCHEMA | No | | | USER | Depends | If AUTHENTICATOR is set to `externalbrowser` this is optional. For native SSO through Okta, set this to the login name for your identity provider (IdP). | | WAREHOUSE | No | | -| CONNECTION_TIMEOUT | No | Total timeout in seconds when connecting to Snowflake. The default is 120 seconds | +| CONNECTION_TIMEOUT | No | Total timeout in seconds when connecting to Snowflake. The default is 300 seconds | +| RETRY_TIMEOUT | No | Total timeout in seconds for supported endpoints of retry policy. The default is 300 seconds. The value can only be increased from the default value or set to 0 for infinite timeout | | MAXHTTPRETRIES | No | Maximum number of times to retry failed HTTP requests (default: 7). You can set `MAXHTTPRETRIES=0` to remove the retry limit, but doing so runs the risk of the .NET driver infinitely retrying failed HTTP calls. | | CLIENT_SESSION_KEEP_ALIVE | No | Whether to keep the current session active after a period of inactivity, or to force the user to login again. If the value is `true`, Snowflake keeps the session active indefinitely, even if there is no activity from the user. If the value is `false`, the user must log in again after four hours of inactivity. The default is `false`. Setting this value overrides the server session property for the current session. | | DISABLERETRY | No | Set this property to `true` to prevent the driver from reconnecting automatically when the connection fails or drops. The default value is `false`. | diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs index ee1442b56..eea852af2 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs @@ -32,7 +32,7 @@ public void TestBasicConnection() conn.Open(); Assert.AreEqual(ConnectionState.Open, conn.State); - Assert.AreEqual(120, conn.ConnectionTimeout); + Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); // Data source is empty string for now Assert.AreEqual("", ((SnowflakeDbConnection)conn).DataSource); @@ -375,38 +375,37 @@ public void TestConnectViaSecureString() [Test] public void TestLoginTimeout() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + using (IDbConnection conn = new MockSnowflakeDbConnection()) { - using (IDbConnection conn = new MockSnowflakeDbConnection()) - { - int timeoutSec = 5; - string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}", - timeoutSec); + int timeoutSec = 5; + string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};maxHttpRetries=0", + timeoutSec); - conn.ConnectionString = loginTimeOut5sec; + conn.ConnectionString = loginTimeOut5sec; - Assert.AreEqual(conn.State, ConnectionState.Closed); - Stopwatch stopwatch = Stopwatch.StartNew(); - try - { - conn.Open(); - Assert.Fail(); - - } - catch (AggregateException e) - { - Assert.AreEqual(SFError.REQUEST_TIMEOUT.GetAttribute().errorCode, - ((SnowflakeDbException)e.InnerException).ErrorCode); - } - stopwatch.Stop(); - int detla = 10; //in case server time slower. - - //Should timeout before the default timeout (120 sec) * 1000 - Assert.Less(stopwatch.ElapsedMilliseconds, 120 * 1000); - // Should timeout after the defined connection timeout - Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, timeoutSec * 1000 - detla); - Assert.AreEqual(5, conn.ConnectionTimeout); + Assert.AreEqual(conn.State, ConnectionState.Closed); + Stopwatch stopwatch = Stopwatch.StartNew(); + try + { + conn.Open(); + Assert.Fail(); + } + catch (AggregateException e) + { + // Jitter can cause the request to reach max number of retries before reaching the timeout + Assert.IsTrue(e.InnerException is TaskCanceledException || + SFError.REQUEST_TIMEOUT.GetAttribute().errorCode == + ((SnowflakeDbException)e.InnerException).ErrorCode); } + stopwatch.Stop(); + int delta = 10; // in case server time slower. + + // Should timeout before the defined timeout plus 1 (buffer time) + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (timeoutSec + 1) * 1000); + // Should timeout after the defined timeout since retry count is infinite + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, timeoutSec * 1000 - delta); + + Assert.AreEqual(timeoutSec, conn.ConnectionTimeout); } } @@ -426,15 +425,58 @@ public void TestLoginWithMaxRetryReached() conn.Open(); Assert.Fail(); } - catch + catch (Exception e) { + // Jitter can cause the request to reach max number of retries before reaching the timeout + Assert.IsTrue(e.InnerException is TaskCanceledException || + SFError.REQUEST_TIMEOUT.GetAttribute().errorCode == + ((SnowflakeDbException)e.InnerException).ErrorCode); } stopwatch.Stop(); - // retry 5 times with backoff 1, 2, 4, 8, 16 seconds - // but should not delay more than another 16 seconds - Assert.Less(stopwatch.ElapsedMilliseconds, 51 * 1000); - Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 30 * 1000); + // retry 5 times with starting backoff of 1 second + // but should not delay more than the max possible seconds after 5 retries + // and should not take less time than the minimum possible seconds after 5 retries + Assert.Less(stopwatch.ElapsedMilliseconds, 79 * 1000); + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 1 * 1000); + } + } + + [Test] + public void TestLoginTimeoutWithRetryTimeoutLesserThanConnectionTimeout() + { + using (IDbConnection conn = new MockSnowflakeDbConnection()) + { + int connectionTimeout = 600; + int retryTimeout = 350; + string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};retry_timeout={1};maxHttpRetries=0", + connectionTimeout, retryTimeout); + + conn.ConnectionString = loginTimeOut5sec; + + Assert.AreEqual(conn.State, ConnectionState.Closed); + Stopwatch stopwatch = Stopwatch.StartNew(); + try + { + conn.Open(); + Assert.Fail(); + } + catch (AggregateException e) + { + // Jitter can cause the request to reach max number of retries before reaching the timeout + Assert.IsTrue(e.InnerException is TaskCanceledException || + SFError.REQUEST_TIMEOUT.GetAttribute().errorCode == + ((SnowflakeDbException)e.InnerException).ErrorCode); + } + stopwatch.Stop(); + int delta = 10; // in case server time slower. + + // Should timeout before the defined timeout plus 1 (buffer time) + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (retryTimeout + 1) * 1000); + // Should timeout after the defined timeout since retry count is infinite + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, retryTimeout * 1000 - delta); + + Assert.AreEqual(retryTimeout, conn.ConnectionTimeout); } } @@ -446,8 +488,8 @@ public void TestDefaultLoginTimeout() { conn.ConnectionString = ConnectionString; - // Default timeout is 120 sec - Assert.AreEqual(120, conn.ConnectionTimeout); + // Default timeout is 300 sec + Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); Assert.AreEqual(conn.State, ConnectionState.Closed); Stopwatch stopwatch = Stopwatch.StartNew(); @@ -464,10 +506,12 @@ public void TestDefaultLoginTimeout() ((SnowflakeDbException)e.InnerException).ErrorCode); stopwatch.Stop(); - // Should timeout after the default timeout (120 sec) - Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 120 * 1000); - // But never more than 16 sec (max backoff) after the default timeout - Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (120 + 16) * 1000); + int delta = 10; // in case server time slower. + + // Should timeout after the default timeout (300 sec) + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, conn.ConnectionTimeout * 1000 - delta); + // But never more because there's no connection timeout remaining + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (conn.ConnectionTimeout + 1) * 1000); } } } @@ -1524,7 +1568,7 @@ public void TestEscapeChar() conn.Open(); Assert.AreEqual(ConnectionState.Open, conn.State); - Assert.AreEqual(120, conn.ConnectionTimeout); + Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); // Data source is empty string for now Assert.AreEqual("", ((SnowflakeDbConnection)conn).DataSource); @@ -1551,7 +1595,7 @@ public void TestEscapeChar1() conn.Open(); Assert.AreEqual(ConnectionState.Open, conn.State); - Assert.AreEqual(120, conn.ConnectionTimeout); + Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); // Data source is empty string for now Assert.AreEqual("", ((SnowflakeDbConnection)conn).DataSource); @@ -1675,15 +1719,14 @@ public void TestCancelLoginBeforeTimeout() Assert.AreEqual(conn.State, ConnectionState.Closed); // At this point the connection string has not been parsed, it will return the // default value - //Assert.AreEqual(120, conn.ConnectionTimeout); + //Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); CancellationTokenSource connectionCancelToken = new CancellationTokenSource(); - Task connectTask = conn.OpenAsync(connectionCancelToken.Token); + Task connectTask = conn.OpenAsync(connectionCancelToken.Token); + + // Sleep for more than the default timeout to make sure there are no false positive) + Thread.Sleep((SFSessionHttpClientProperties.s_retryTimeoutDefault + 10) * 1000); - // Sleep for 130 sec (more than the default connection timeout and the httpclient - // timeout to make sure there are no false positive ) - Thread.Sleep(130*1000); - Assert.AreEqual(ConnectionState.Connecting, conn.State); // Cancel the connection because it will never succeed since there is no @@ -1704,47 +1747,82 @@ public void TestCancelLoginBeforeTimeout() } Assert.AreEqual(ConnectionState.Closed, conn.State); - Assert.AreEqual(0, conn.ConnectionTimeout); + Assert.AreEqual(timeoutSec, conn.ConnectionTimeout); } } [Test] public void TestAsyncLoginTimeout() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + using (var conn = new MockSnowflakeDbConnection()) { - using (var conn = new MockSnowflakeDbConnection()) + int timeoutSec = 5; + string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};maxHttpRetries=0", + timeoutSec); + conn.ConnectionString = loginTimeOut5sec; + + Assert.AreEqual(conn.State, ConnectionState.Closed); + + Stopwatch stopwatch = Stopwatch.StartNew(); + try { - int timeoutSec = 5; - string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}", - timeoutSec); - conn.ConnectionString = loginTimeOut5sec; + Task connectTask = conn.OpenAsync(CancellationToken.None); + connectTask.Wait(); + } + catch (AggregateException e) + { + Assert.AreEqual(SFError.INTERNAL_ERROR.GetAttribute().errorCode, + ((SnowflakeDbException)e.InnerException).ErrorCode); - Assert.AreEqual(conn.State, ConnectionState.Closed); + } + stopwatch.Stop(); + int delta = 10; // in case server time slower. - Stopwatch stopwatch = Stopwatch.StartNew(); - try - { - Task connectTask = conn.OpenAsync(CancellationToken.None); - connectTask.Wait(); - } - catch (AggregateException e) - { - Assert.AreEqual(SFError.INTERNAL_ERROR.GetAttribute().errorCode, - ((SnowflakeDbException)e.InnerException).ErrorCode); + // Should timeout after the defined timeout since retry count is infinite + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, timeoutSec * 1000 - delta); + // But never more than 1 sec (buffer time) after the defined timeout + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (timeoutSec + 1) * 1000); - } - stopwatch.Stop(); - int detla = 10; //in case server time slower. + Assert.AreEqual(ConnectionState.Closed, conn.State); + Assert.AreEqual(timeoutSec, conn.ConnectionTimeout); + } + } + + [Test] + public void TestAsyncLoginTimeoutWithRetryTimeoutLesserThanConnectionTimeout() + { + using (var conn = new MockSnowflakeDbConnection()) + { + int connectionTimeout = 600; + int retryTimeout = 350; + string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};retry_timeout={1};maxHttpRetries=0", + connectionTimeout, retryTimeout); + conn.ConnectionString = loginTimeOut5sec; - // Should timeout after 5sec - Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 5 * 1000 - detla); - // But never more than 1 sec (max backoff) after the default timeout - Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (6) * 1000); + Assert.AreEqual(conn.State, ConnectionState.Closed); + + Stopwatch stopwatch = Stopwatch.StartNew(); + try + { + Task connectTask = conn.OpenAsync(CancellationToken.None); + connectTask.Wait(); + } + catch (AggregateException e) + { + Assert.AreEqual(SFError.INTERNAL_ERROR.GetAttribute().errorCode, + ((SnowflakeDbException)e.InnerException).ErrorCode); - Assert.AreEqual(ConnectionState.Closed, conn.State); - Assert.AreEqual(5, conn.ConnectionTimeout); } + stopwatch.Stop(); + int delta = 10; // in case server time slower. + + // Should timeout after the defined timeout since retry count is infinite + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, retryTimeout * 1000 - delta); + // But never more than 1 sec (buffer time) after the defined timeout + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (retryTimeout + 1) * 1000); + + Assert.AreEqual(ConnectionState.Closed, conn.State); + Assert.AreEqual(retryTimeout, conn.ConnectionTimeout); } } @@ -1770,15 +1848,15 @@ public void TestAsyncDefaultLoginTimeout() ((SnowflakeDbException)e.InnerException).ErrorCode); } stopwatch.Stop(); - int detla = 10; //in case server time slower. + int delta = 10; // in case server time slower. - // Should timeout after the default timeout (120 sec) - Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 120 * 1000 - detla); - // But never more than 16 sec (max backoff) after the default timeout - Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (120 + 16) * 1000); + // Should timeout after the default timeout (300 sec) + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, conn.ConnectionTimeout * 1000 - delta); + // But never more because there's no connection timeout remaining + Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (conn.ConnectionTimeout + 1) * 1000); Assert.AreEqual(ConnectionState.Closed, conn.State); - Assert.AreEqual(120, conn.ConnectionTimeout); + Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout); } } diff --git a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs index 111094b74..58bf90b46 100755 --- a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs @@ -16,6 +16,8 @@ namespace Snowflake.Data.Tests.IntegrationTests using System.Diagnostics; using System.Collections.Generic; using System.Globalization; + using Snowflake.Data.Tests.Mock; + using Snowflake.Data.Core; [TestFixture] class SFDbCommandITAsync : SFBaseTestAsync @@ -90,6 +92,44 @@ public void TestCancelExecuteAsync() } } + [Test] + public void TestExecuteAsyncWithMaxRetryReached() + { + var mockRestRequester = new MockRetryUntilRestTimeoutRestRequester() + { + _forceTimeoutForNonLoginRequestsOnly = true + }; + + using (DbConnection conn = new MockSnowflakeDbConnection(mockRestRequester)) + { + string maxRetryConnStr = ConnectionString + "maxHttpRetries=5"; + + conn.ConnectionString = maxRetryConnStr; + conn.Open(); + + Stopwatch stopwatch = Stopwatch.StartNew(); + try + { + using (DbCommand command = conn.CreateCommand()) + { + command.CommandText = "select 1;"; + Task t = command.ExecuteScalarAsync(); + t.Wait(); + } + Assert.Fail(); + } + catch (Exception e) + { + Assert.IsInstanceOf(e.InnerException); + } + stopwatch.Stop(); + + // retry 5 times with backoff 1, 2, 4, 8, 16 seconds + // but should not delay more than another 16 seconds + Assert.Less(stopwatch.ElapsedMilliseconds, 51 * 1000); + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 30 * 1000); + } + } } [TestFixture] @@ -544,6 +584,44 @@ public void TestExecuteScalarNull() } } + [Test] + public void TestExecuteWithMaxRetryReached() + { + var mockRestRequester = new MockRetryUntilRestTimeoutRestRequester() + { + _forceTimeoutForNonLoginRequestsOnly = true + }; + + using (IDbConnection conn = new MockSnowflakeDbConnection(mockRestRequester)) + { + string maxRetryConnStr = ConnectionString + "maxHttpRetries=5"; + + conn.ConnectionString = maxRetryConnStr; + conn.Open(); + + Stopwatch stopwatch = Stopwatch.StartNew(); + try + { + using (IDbCommand command = conn.CreateCommand()) + { + command.CommandText = "select 1;"; + command.ExecuteScalar(); + } + Assert.Fail(); + } + catch (Exception e) + { + Assert.IsInstanceOf(e.InnerException); + } + stopwatch.Stop(); + + // retry 5 times with backoff 1, 2, 4, 8, 16 seconds + // but should not delay more than another 16 seconds + Assert.Less(stopwatch.ElapsedMilliseconds, 51 * 1000); + Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 30 * 1000); + } + } + [Test] public void TestCreateCommandBeforeOpeningConnection() { diff --git a/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs b/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs index c9f9a665a..17b0ffcfc 100644 --- a/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs +++ b/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs @@ -15,6 +15,8 @@ namespace Snowflake.Data.Tests.Mock class MockRetryUntilRestTimeoutRestRequester : RestRequester, IMockRestRequester { + internal bool _forceTimeoutForNonLoginRequestsOnly = false; + public MockRetryUntilRestTimeoutRestRequester() : base(null) { // Does nothing @@ -30,8 +32,13 @@ protected override async Task SendAsync(HttpRequestMessage CancellationToken externalCancellationToken, string sid = "") { - // Override the http timeout and set to 1ms to force all http request to timeout and retry - message.Properties[BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY] = TimeSpan.FromTicks(0); + if (!_forceTimeoutForNonLoginRequestsOnly || + _forceTimeoutForNonLoginRequestsOnly && !message.RequestUri.AbsolutePath.Equals(RestPath.SF_LOGIN_PATH)) + { + // Override the http timeout and set to 1ms to force all http request to timeout and retry + message.Properties[BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY] = TimeSpan.FromTicks(0); + } + return await (base.SendAsync(message, restTimeout, externalCancellationToken).ConfigureAwait(false)); } } diff --git a/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs b/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs index aad4fe245..bf8063a7f 100644 --- a/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs +++ b/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs @@ -11,6 +11,7 @@ namespace Snowflake.Data.Tests.UnitTests using RichardSzalay.MockHttp; using System.Threading.Tasks; using System.Net; + using System; [TestFixture] class HttpUtilTest @@ -39,6 +40,49 @@ public async Task TestIsRetryableHTTPCode(HttpStatusCode statusCode, bool forceR Assert.AreEqual(expectedIsRetryable, actualIsRetryable); } + // Parameters: request url, expected value + [TestCase("https://test.snowflakecomputing.com/session/v1/login-request", true)] + [TestCase("https://test.snowflakecomputing.com/session/authenticator-request", true)] + [TestCase("https://test.snowflakecomputing.com/session/token-request", true)] + [TestCase("https://test.snowflakecomputing.com/queries/v1/query-request", false)] + [Test] + public void TestIsLoginUrl(string requestUrl, bool expectedIsLoginEndpoint) + { + // given + var uri = new Uri(requestUrl); + + // when + bool isLoginEndpoint = HttpUtil.IsLoginEndpoint(uri.AbsolutePath); + + // then + Assert.AreEqual(expectedIsLoginEndpoint, isLoginEndpoint); + } + + // Parameters: time in seconds + [TestCase(4)] + [TestCase(8)] + [TestCase(16)] + [TestCase(32)] + [TestCase(64)] + [TestCase(128)] + [Test] + public void TestGetJitter(int seconds) + { + // given + var lowerBound = -(seconds / 2); + var upperBound = seconds / 2; + + double jitter; + for (var i = 0; i < 10; i++) + { + // when + jitter = HttpUtil.GetJitter(seconds); + + // then + Assert.IsTrue(jitter >= lowerBound && jitter <= upperBound); + } + } + [Test] public void ShouldCreateHttpClientHandlerWithProxy() { diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs index cdf6296e2..dd31f1c16 100644 --- a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs @@ -48,7 +48,7 @@ public static IEnumerable ConnectionStringTestCases() string defHost = "testaccount.snowflakecomputing.com"; string defAuthenticator = "snowflake"; string defScheme = "https"; - string defConnectionTimeout = "120"; + string defConnectionTimeout = "300"; string defBrowserResponseTime = "120"; string defPassword = "123"; string defPort = "443"; @@ -57,6 +57,7 @@ public static IEnumerable ConnectionStringTestCases() string defProxyPort = "1234"; string defNonProxyHosts = "localhost"; + string defRetryTimeout = "300"; string defMaxHttpRetries = "7"; string defIncludeRetryReason = "true"; string defDisableQueryContextCache = "false"; @@ -82,6 +83,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CLIENT_SESSION_KEEP_ALIVE, "false" }, { SFSessionProperty.FORCEPARSEERROR, "false" }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, defDisableQueryContextCache } @@ -107,6 +109,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CLIENT_SESSION_KEEP_ALIVE, "false" }, { SFSessionProperty.FORCEPARSEERROR, "false" }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, "180" }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, defDisableQueryContextCache } @@ -135,6 +138,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.PROXYPORT, defProxyPort }, { SFSessionProperty.NONPROXYHOSTS, defNonProxyHosts }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, defDisableQueryContextCache } @@ -165,6 +169,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.PROXYPORT, defProxyPort }, { SFSessionProperty.NONPROXYHOSTS, defNonProxyHosts }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, defDisableQueryContextCache } @@ -193,6 +198,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CLIENT_SESSION_KEEP_ALIVE, "false" }, { SFSessionProperty.FORCEPARSEERROR, "false" }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.FILE_TRANSFER_MEMORY_THRESHOLD, "25" }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, @@ -220,6 +226,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CLIENT_SESSION_KEEP_ALIVE, "false" }, { SFSessionProperty.FORCEPARSEERROR, "false" }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, "false" }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, defDisableQueryContextCache } @@ -245,6 +252,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CLIENT_SESSION_KEEP_ALIVE, "false" }, { SFSessionProperty.FORCEPARSEERROR, "false" }, { SFSessionProperty.BROWSER_RESPONSE_TIMEOUT, defBrowserResponseTime }, + { SFSessionProperty.RETRY_TIMEOUT, defRetryTimeout }, { SFSessionProperty.MAXHTTPRETRIES, defMaxHttpRetries }, { SFSessionProperty.INCLUDERETRYREASON, defIncludeRetryReason }, { SFSessionProperty.DISABLEQUERYCONTEXTCACHE, "true" } diff --git a/Snowflake.Data.Tests/UnitTests/Session/SFHttpClientPropertiesTest.cs b/Snowflake.Data.Tests/UnitTests/Session/SFHttpClientPropertiesTest.cs index ea2b5d336..ea6deb5ca 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/SFHttpClientPropertiesTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/SFHttpClientPropertiesTest.cs @@ -32,10 +32,11 @@ public void ShouldConvertToMapOnly2Properties( { validateDefaultParameters = validateDefaultParameters, clientSessionKeepAlive = clientSessionKeepAlive, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = false, + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, maxHttpRetries = 7, proxyProperties = proxyProperties }; @@ -69,6 +70,7 @@ public void ShouldBuildHttpClientConfig() insecureMode = TestDataGenarator.NextBool(), disableRetry = TestDataGenarator.NextBool(), forceRetryOn404 = TestDataGenarator.NextBool(), + retryTimeout = TestDataGenarator.NextInt(300, 600), maxHttpRetries = TestDataGenarator.NextInt(0, 15), proxyProperties = proxyProperties }; @@ -102,6 +104,7 @@ public void ShouldExtractProperties(PropertiesTestCase testCase) // when var extractedProperties = extractor.ExtractProperties(properties); + extractedProperties.CheckPropertiesAreValid(); // then Assert.AreEqual(testCase.expectedProperties.validateDefaultParameters, extractedProperties.validateDefaultParameters); @@ -110,6 +113,7 @@ public void ShouldExtractProperties(PropertiesTestCase testCase) Assert.AreEqual(testCase.expectedProperties.insecureMode, extractedProperties.insecureMode); Assert.AreEqual(testCase.expectedProperties.disableRetry, extractedProperties.disableRetry); Assert.AreEqual(testCase.expectedProperties.forceRetryOn404, extractedProperties.forceRetryOn404); + Assert.AreEqual(testCase.expectedProperties.retryTimeout, extractedProperties.retryTimeout); Assert.AreEqual(testCase.expectedProperties.maxHttpRetries, extractedProperties.maxHttpRetries); Assert.AreEqual(proxyProperties, extractedProperties.proxyProperties); proxyExtractorMock.Verify(e => e.ExtractProperties(properties), Times.Once); @@ -124,11 +128,12 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = true, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithValidateDefaultParametersChanged = new PropertiesTestCase() @@ -138,11 +143,12 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = false, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithClientSessionKeepAliveChanged = new PropertiesTestCase() @@ -152,11 +158,12 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = true, clientSessionKeepAlive = true, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithTimeoutChanged = new PropertiesTestCase() @@ -170,7 +177,8 @@ public static IEnumerable PropertiesProvider() insecureMode = false, disableRetry = false, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithInsecureModeChanged = new PropertiesTestCase() @@ -180,11 +188,12 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = true, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = true, disableRetry = false, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithDisableRetryChanged = new PropertiesTestCase() @@ -194,11 +203,12 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = true, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = true, forceRetryOn404 = false, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; var propertiesWithForceRetryOn404Changed = new PropertiesTestCase() @@ -208,27 +218,104 @@ public static IEnumerable PropertiesProvider() { validateDefaultParameters = true, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = true, - maxHttpRetries = 7 + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault } }; - var propertiesWithMaxHttpRetiesChanged = new PropertiesTestCase() + var propertiesWithRetryTimeoutChangedToAValueAbove300 = new PropertiesTestCase() + { + conectionString = "account=test;user=test;password=test;retry_timeout=600", + expectedProperties = new SFSessionHttpClientProperties() + { + validateDefaultParameters = true, + clientSessionKeepAlive = false, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, + insecureMode = false, + disableRetry = false, + forceRetryOn404 = false, + retryTimeout = 600, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault + } + }; + var propertiesWithRetryTimeoutChangedToAValueBelow300 = new PropertiesTestCase() + { + conectionString = "account=test;user=test;password=test;retry_timeout=15", + expectedProperties = new SFSessionHttpClientProperties() + { + validateDefaultParameters = true, + clientSessionKeepAlive = false, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, + insecureMode = false, + disableRetry = false, + forceRetryOn404 = false, + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault + } + }; + var propertiesWithRetryTimeoutChangedToZero = new PropertiesTestCase() + { + conectionString = "account=test;user=test;password=test;retry_timeout=0", + expectedProperties = new SFSessionHttpClientProperties() + { + validateDefaultParameters = true, + clientSessionKeepAlive = false, + timeoutInSec = 0, + insecureMode = false, + disableRetry = false, + forceRetryOn404 = false, + retryTimeout = 0, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault + } + }; + var propertiesWithMaxHttpRetriesChangedToAValueAbove7 = new PropertiesTestCase() { conectionString = "account=test;user=test;password=test;maxHttpRetries=10", expectedProperties = new SFSessionHttpClientProperties() { validateDefaultParameters = true, clientSessionKeepAlive = false, - timeoutInSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, insecureMode = false, disableRetry = false, forceRetryOn404 = false, + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, maxHttpRetries = 10 } }; + var propertiesWithMaxHttpRetriesChangedToAValueBelow7 = new PropertiesTestCase() + { + conectionString = "account=test;user=test;password=test;maxHttpRetries=5", + expectedProperties = new SFSessionHttpClientProperties() + { + validateDefaultParameters = true, + clientSessionKeepAlive = false, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, + insecureMode = false, + disableRetry = false, + forceRetryOn404 = false, + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = SFSessionHttpClientProperties.s_maxHttpRetriesDefault + } + }; + var propertiesWithMaxHttpRetriesChangedToZero = new PropertiesTestCase() + { + conectionString = "account=test;user=test;password=test;maxHttpRetries=0", + expectedProperties = new SFSessionHttpClientProperties() + { + validateDefaultParameters = true, + clientSessionKeepAlive = false, + timeoutInSec = SFSessionHttpClientProperties.s_retryTimeoutDefault, + insecureMode = false, + disableRetry = false, + forceRetryOn404 = false, + retryTimeout = SFSessionHttpClientProperties.s_retryTimeoutDefault, + maxHttpRetries = 0 + } + }; return new [] { defaultProperties, @@ -238,7 +325,12 @@ public static IEnumerable PropertiesProvider() propertiesWithInsecureModeChanged, propertiesWithDisableRetryChanged, propertiesWithForceRetryOn404Changed, - propertiesWithMaxHttpRetiesChanged + propertiesWithRetryTimeoutChangedToAValueAbove300, + propertiesWithRetryTimeoutChangedToAValueBelow300, + propertiesWithRetryTimeoutChangedToZero, + propertiesWithMaxHttpRetriesChangedToAValueAbove7, + propertiesWithMaxHttpRetriesChangedToAValueBelow7, + propertiesWithMaxHttpRetriesChangedToZero }; } diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index 77d3807ff..b48d5a3a9 100755 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -13,6 +13,7 @@ using System.Web; using System.Security.Authentication; using System.Runtime.InteropServices; +using System.Linq; namespace Snowflake.Data.Core { @@ -73,8 +74,17 @@ public HttpClientConfig( public sealed class HttpUtil { static internal readonly int MAX_BACKOFF = 16; + private static readonly int s_baseBackOffTime = 1; + private static readonly int s_exponentialFactor = 2; private static readonly SFLogger logger = SFLoggerFactory.GetLogger(); + private static readonly List s_supportedEndpointsForRetryPolicy = new List + { + RestPath.SF_LOGIN_PATH, + RestPath.SF_AUTHENTICATOR_REQUEST_PATH, + RestPath.SF_TOKEN_REQUEST_PATH + }; + private HttpUtil() { // This value is used by AWS SDK and can cause deadlock, @@ -317,14 +327,14 @@ private class RetryHandler : DelegatingHandler private bool disableRetry; private bool forceRetryOn404; - private int maxRetry; + private int maxRetryCount; private bool includeRetryReason; - internal RetryHandler(HttpMessageHandler innerHandler, bool disableRetry, bool forceRetryOn404, int maxRetry, bool includeRetryReason) : base(innerHandler) + internal RetryHandler(HttpMessageHandler innerHandler, bool disableRetry, bool forceRetryOn404, int maxRetryCount, bool includeRetryReason) : base(innerHandler) { this.disableRetry = disableRetry; this.forceRetryOn404 = forceRetryOn404; - this.maxRetry = maxRetry; + this.maxRetryCount = maxRetryCount; this.includeRetryReason = includeRetryReason; } @@ -332,7 +342,8 @@ protected override async Task SendAsync(HttpRequestMessage CancellationToken cancellationToken) { HttpResponseMessage response = null; - int backOffInSec = 1; + bool isLoginRequest = IsLoginEndpoint(requestMessage.RequestUri.AbsolutePath); + int backOffInSec = s_baseBackOffTime; int totalRetryTime = 0; ServicePoint p = ServicePointManager.FindServicePoint(requestMessage.RequestUri); @@ -397,6 +408,7 @@ protected override async Task SendAsync(HttpRequestMessage } int errorReason = 0; + if (response != null) { if (response.IsSuccessStatusCode) @@ -423,14 +435,14 @@ protected override async Task SendAsync(HttpRequestMessage } retryCount++; - if ((maxRetry > 0) && (retryCount > maxRetry)) + if ((maxRetryCount > 0) && (retryCount > maxRetryCount)) { - logger.Debug($"stop retry as maxHttpRetries {maxRetry} reached"); + logger.Debug($"stop retry as maxHttpRetries {maxRetryCount} reached"); if (response != null) { return response; } - throw new OperationCanceledException($"http request failed and max retry {maxRetry} reached"); + throw new OperationCanceledException($"http request failed and max retry {maxRetryCount} reached"); } // Disposing of the response if not null now that we don't need it anymore @@ -442,16 +454,29 @@ protected override async Task SendAsync(HttpRequestMessage await Task.Delay(TimeSpan.FromSeconds(backOffInSec), cancellationToken).ConfigureAwait(false); totalRetryTime += backOffInSec; - // Set next backoff time - backOffInSec = backOffInSec >= MAX_BACKOFF ? - MAX_BACKOFF : backOffInSec * 2; + + var jitter = GetJitter(backOffInSec); + + // Set backoff time + if (isLoginRequest) + { + // Choose between previous sleep time and new base sleep time for login requests + backOffInSec = (int)ChooseRandom( + backOffInSec + jitter, + Math.Pow(s_exponentialFactor, retryCount) + jitter); + } + else if (backOffInSec < MAX_BACKOFF) + { + // Multiply sleep by 2 for non-login requests + backOffInSec *= 2; + } if ((restTimeout.TotalSeconds > 0) && (totalRetryTime + backOffInSec > restTimeout.TotalSeconds)) { // No need to wait more than necessary if it can be avoided. // If the rest timeout will be reached before the next back-off, - // use a smaller one to give the Rest request a chance to timeout early - backOffInSec = Math.Max(1, (int)restTimeout.TotalSeconds - totalRetryTime - 1); + // then use the remaining connection timeout + backOffInSec = Math.Min(backOffInSec, (int)restTimeout.TotalSeconds - totalRetryTime); } } } @@ -474,6 +499,41 @@ static public bool isRetryableHTTPCode(int statusCode, bool forceRetryOn404) // Too many requests (statusCode == 429); } + + /// + /// Get the jitter amount based on current wait time. + /// + /// The current retry backoff time. + /// The new jitter amount. + static internal double GetJitter(double curWaitTime) + { + double multiplicationFactor = ChooseRandom(-1, 1); + double jitterAmount = 0.5 * curWaitTime * multiplicationFactor; + return jitterAmount; + } + + /// + /// Randomly generates a number between a given range. + /// + /// The min range (inclusive). + /// The max range (inclusive). + /// The random number. + static double ChooseRandom(double min, double max) + { + var next = new Random().NextDouble(); + + return min + (next * (max - min)); + } + + /// + /// Checks if the endpoint is a login request. + /// + /// The endpoint to check. + /// True if the endpoint is a login request, false otherwise. + static internal bool IsLoginEndpoint(string endpoint) + { + return null != s_supportedEndpointsForRetryPolicy.FirstOrDefault(ep => endpoint.Equals(ep)); + } } } diff --git a/Snowflake.Data/Core/RestRequest.cs b/Snowflake.Data/Core/RestRequest.cs index 77a88323b..bc4ec8d21 100644 --- a/Snowflake.Data/Core/RestRequest.cs +++ b/Snowflake.Data/Core/RestRequest.cs @@ -108,6 +108,9 @@ internal class SFRestRequest : BaseRestRequest, IRestRequest private const string SF_AUTHORIZATION_HEADER = "Authorization"; private const string SF_SERVICE_NAME_HEADER = "X-Snowflake-Service"; + private const string ClientAppId = "CLIENT_APP_ID"; + private const string ClientAppVersion = "CLIENT_APP_VERSION"; + internal SFRestRequest() : base() { RestTimeout = TimeSpan.FromSeconds(DEFAULT_REST_RETRY_SECONDS_TIMEOUT); @@ -124,6 +127,8 @@ internal SFRestRequest() : base() internal bool isPutGet { get; set; } + internal bool _isLogin { get; set; } + public override string ToString() { return String.Format("SFRestRequest {{url: {0}, request body: {1} }}", Url.ToString(), @@ -158,6 +163,12 @@ HttpRequestMessage IRestRequest.ToRequestMessage(HttpMethod method) message.Headers.Accept.Add(applicationSnowflake); } + if (_isLogin) + { + message.Headers.Add(ClientAppId, SFEnvironment.DriverName); + message.Headers.Add(ClientAppVersion, SFEnvironment.DriverVersion); + } + message.Headers.UserAgent.Add(new ProductInfoHeaderValue(SFEnvironment.DriverName, SFEnvironment.DriverVersion)); message.Headers.UserAgent.Add(new ProductInfoHeaderValue(osInfo)); message.Headers.UserAgent.Add(new ProductInfoHeaderValue( diff --git a/Snowflake.Data/Core/Session/SFSession.cs b/Snowflake.Data/Core/Session/SFSession.cs index 49586c08d..d153ffff4 100755 --- a/Snowflake.Data/Core/Session/SFSession.cs +++ b/Snowflake.Data/Core/Session/SFSession.cs @@ -157,7 +157,7 @@ internal SFSession( InsecureMode = extractedProperties.insecureMode; _HttpClient = HttpUtil.Instance.GetHttpClient(httpClientConfig); restRequester = new RestRequester(_HttpClient); - extractedProperties.WarnOnTimeout(); + extractedProperties.CheckPropertiesAreValid(); connectionTimeout = extractedProperties.TimeoutDuration(); properties.TryGetValue(SFSessionProperty.CLIENT_CONFIG_FILE, out var easyLoggingConfigFile); _easyLoggingStarter.Init(easyLoggingConfigFile); @@ -362,7 +362,8 @@ internal SFRestRequest getRenewSessionRequest() jsonBody = postBody, Url = BuildUri(RestPath.SF_TOKEN_REQUEST_PATH, parameters), authorizationToken = string.Format(SF_AUTHORIZATION_SNOWFLAKE_FMT, masterToken), - RestTimeout = Timeout.InfiniteTimeSpan + RestTimeout = Timeout.InfiniteTimeSpan, + _isLogin = true }; } @@ -374,6 +375,7 @@ internal SFRestRequest BuildTimeoutRestRequest(Uri uri, Object body) Url = uri, authorizationToken = SF_AUTHORIZATION_BASIC, RestTimeout = connectionTimeout, + _isLogin = true }; } diff --git a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs index 57a6cfe14..b7466a362 100644 --- a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs +++ b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs @@ -8,7 +8,8 @@ namespace Snowflake.Data.Core internal class SFSessionHttpClientProperties { - private static int recommendedMinTimeoutSec = BaseRestRequest.DEFAULT_REST_RETRY_SECONDS_TIMEOUT; + internal static readonly int s_maxHttpRetriesDefault = 7; + internal static readonly int s_retryTimeoutDefault = 300; private static readonly SFLogger logger = SFLoggerFactory.GetLogger(); internal bool validateDefaultParameters; @@ -17,22 +18,53 @@ internal class SFSessionHttpClientProperties internal bool insecureMode; internal bool disableRetry; internal bool forceRetryOn404; + internal int retryTimeout; internal int maxHttpRetries; internal bool includeRetryReason; internal SFSessionHttpClientProxyProperties proxyProperties; - internal void WarnOnTimeout() + internal void CheckPropertiesAreValid() { - if (timeoutInSec < recommendedMinTimeoutSec) + if (timeoutInSec < s_retryTimeoutDefault) { logger.Warn($"Connection timeout provided is less than recommended minimum value of" + - $" {recommendedMinTimeoutSec}"); + $" {s_retryTimeoutDefault}"); } if (timeoutInSec < 0) { logger.Warn($"Connection timeout provided is negative. Timeout will be infinite."); } + + if (retryTimeout > 0 && retryTimeout < s_retryTimeoutDefault) + { + logger.Warn($"Max retry timeout provided is less than the allowed minimum value of" + + $" {s_retryTimeoutDefault}"); + + retryTimeout = s_retryTimeoutDefault; + } + else if (retryTimeout == 0) + { + logger.Warn($"Max retry timeout provided is 0. Timeout will be infinite"); + } + + // Use the shorter timeout between CONNECTION_TIMEOUT and RETRY_TIMEOUT + if (retryTimeout < timeoutInSec) + { + timeoutInSec = retryTimeout; + } + + if (maxHttpRetries > 0 && maxHttpRetries < s_maxHttpRetriesDefault) + { + logger.Warn($"Max retry count provided is less than the allowed minimum value of" + + $" {s_maxHttpRetriesDefault}"); + + maxHttpRetries = s_maxHttpRetriesDefault; + } + else if (maxHttpRetries == 0) + { + logger.Warn($"Max retry count provided is 0. Retry count will be infinite"); + } } internal TimeSpan TimeoutDuration() @@ -87,6 +119,7 @@ public SFSessionHttpClientProperties ExtractProperties(SFSessionProperties prope insecureMode = Boolean.Parse(propertiesDictionary[SFSessionProperty.INSECUREMODE]), disableRetry = Boolean.Parse(propertiesDictionary[SFSessionProperty.DISABLERETRY]), forceRetryOn404 = Boolean.Parse(propertiesDictionary[SFSessionProperty.FORCERETRYON404]), + retryTimeout = int.Parse(propertiesDictionary[SFSessionProperty.RETRY_TIMEOUT]), maxHttpRetries = int.Parse(propertiesDictionary[SFSessionProperty.MAXHTTPRETRIES]), includeRetryReason = Boolean.Parse(propertiesDictionary[SFSessionProperty.INCLUDERETRYREASON]), proxyProperties = proxyPropertiesExtractor.ExtractProperties(propertiesDictionary) diff --git a/Snowflake.Data/Core/Session/SFSessionProperty.cs b/Snowflake.Data/Core/Session/SFSessionProperty.cs index ade4d516c..28ef2fa7e 100755 --- a/Snowflake.Data/Core/Session/SFSessionProperty.cs +++ b/Snowflake.Data/Core/Session/SFSessionProperty.cs @@ -36,7 +36,7 @@ internal enum SFSessionProperty USER, [SFSessionPropertyAttr(required = false)] WAREHOUSE, - [SFSessionPropertyAttr(required = false, defaultValue = "120")] + [SFSessionPropertyAttr(required = false, defaultValue = "300")] CONNECTION_TIMEOUT, [SFSessionPropertyAttr(required = false, defaultValue = "snowflake")] AUTHENTICATOR, @@ -78,6 +78,8 @@ internal enum SFSessionProperty FORCEPARSEERROR, [SFSessionPropertyAttr(required = false, defaultValue = "120")] BROWSER_RESPONSE_TIMEOUT, + [SFSessionPropertyAttr(required = false, defaultValue = "300")] + RETRY_TIMEOUT, [SFSessionPropertyAttr(required = false, defaultValue = "7")] MAXHTTPRETRIES, [SFSessionPropertyAttr(required = false)] @@ -266,7 +268,6 @@ internal static SFSessionProperties parseConnectionString(String connectionStrin // passed on for account_name properties[SFSessionProperty.ACCOUNT] = properties[SFSessionProperty.ACCOUNT].Split('.')[0]; - return properties; }