Skip to content

Commit

Permalink
SNOW-878067: Update retry logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-ext-simba-lf
sfc-gh-ext-simba-lf committed Nov 3, 2023
1 parent da0d1c6 commit 8007e5e
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 96 deletions.
62 changes: 31 additions & 31 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void TestBasicConnection()
conn.Open();
Assert.AreEqual(ConnectionState.Open, conn.State);

Assert.AreEqual(SFSessionHttpClientProperties.s_connectionTimeoutDefault, conn.ConnectionTimeout);
Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout);
// Data source is empty string for now
Assert.AreEqual("", ((SnowflakeDbConnection)conn).DataSource);

Expand Down Expand Up @@ -379,8 +379,7 @@ public void TestLoginTimeout()
{
using (IDbConnection conn = new MockSnowflakeDbConnection())
{
// Connection timeout can only be increased
int timeoutSec = 5 + SFSessionHttpClientProperties.s_connectionTimeoutDefault;
int timeoutSec = 5;
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}",
timeoutSec);

Expand All @@ -392,7 +391,6 @@ public void TestLoginTimeout()
{
conn.Open();
Assert.Fail();

}
catch (AggregateException e)
{
Expand All @@ -404,8 +402,8 @@ public void TestLoginTimeout()
stopwatch.Stop();
int delta = 100; // in case server time slower.

// Should timeout after the minimum possible timeout with jitter: 14 seconds (50% of 4 * 2) after 7 retries
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 14 * 1000);
// Should timeout after the minimum possible timeout with jitter
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 1 * 1000);
// Should timeout before the defined connection timeout
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, timeoutSec * 1000 + delta);
Assert.AreEqual(timeoutSec, conn.ConnectionTimeout);
Expand Down Expand Up @@ -438,11 +436,11 @@ public void TestLoginWithMaxRetryReached()
}
stopwatch.Stop();

// retry 5 times with starting backoff of 4 seconds with jitter
// but should not delay more than the default connection timeout of 300 seconds
// and should not take less time than 10 (with max negative jitter for 5 retries) seconds
Assert.Less(stopwatch.ElapsedMilliseconds, SFSessionHttpClientProperties.s_connectionTimeoutDefault * 1000);
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 10 * 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);
}
}

Expand All @@ -454,8 +452,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();
Expand All @@ -472,10 +470,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 = 100; // 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 * 1000 + delta);
}
}
}
Expand Down Expand Up @@ -1532,7 +1532,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);

Expand All @@ -1559,7 +1559,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);

Expand Down Expand Up @@ -1673,8 +1673,8 @@ public void TestCancelLoginBeforeTimeout()
{
using (var conn = new MockSnowflakeDbConnection())
{
// Add 10 seconds to default timeout
int timeoutSec = 10 + SFSessionHttpClientProperties.s_connectionTimeoutDefault;
// No timeout
int timeoutSec = 0;
string infiniteLoginTimeOut = String.Format(ConnectionString + ";connection_timeout={0};maxHttpRetries=0",
timeoutSec);

Expand All @@ -1683,13 +1683,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);

// Sleep for the specified timeout minus 10 seconds (just before the actual timeout is reached)
Thread.Sleep((timeoutSec - 10) * 1000);
// Sleep for the default timeout minus 10 seconds (just before the actual timeout is reached)
// to make sure there are no false positive
Thread.Sleep((SFSessionHttpClientProperties.s_retryTimeoutDefault - 10) * 1000);

Assert.AreEqual(ConnectionState.Connecting, conn.State);

Expand Down Expand Up @@ -1722,8 +1723,7 @@ public void TestAsyncLoginTimeout()
{
using (var conn = new MockSnowflakeDbConnection())
{
// Connection timeout can only be increased
int timeoutSec = 5 + SFSessionHttpClientProperties.s_connectionTimeoutDefault;
int timeoutSec = 5;
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}",
timeoutSec);
conn.ConnectionString = loginTimeOut5sec;
Expand All @@ -1745,8 +1745,8 @@ public void TestAsyncLoginTimeout()
stopwatch.Stop();
int delta = 100; // in case server time slower.

// Should timeout after the minimum possible timeout with jitter: 14 seconds (50% of 4 * 2) after 7 retries
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 14 * 1000);
// Should timeout after the minimum possible timeout with jitter
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, 1 * 1000);
// Should timeout before the defined connection timeout
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, timeoutSec * 1000 + delta);

Expand Down Expand Up @@ -1781,12 +1781,12 @@ public void TestAsyncDefaultLoginTimeout()
int delta = 100; // in case server time slower.

// Should timeout after the default timeout (300 sec)
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, SFSessionHttpClientProperties.s_connectionTimeoutDefault * 1000 - delta);
Assert.GreaterOrEqual(stopwatch.ElapsedMilliseconds, conn.ConnectionTimeout * 1000 - delta);
// But never more because there's no connection timeout remaining
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, SFSessionHttpClientProperties.s_connectionTimeoutDefault * 1000 + delta);
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, conn.ConnectionTimeout * 1000 + delta);

Assert.AreEqual(ConnectionState.Closed, conn.State);
Assert.AreEqual(SFSessionHttpClientProperties.s_connectionTimeoutDefault, conn.ConnectionTimeout);
Assert.AreEqual(SFSessionHttpClientProperties.s_retryTimeoutDefault, conn.ConnectionTimeout);
}
}

Expand Down
2 changes: 2 additions & 0 deletions Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public void ShouldCreateHttpClientHandlerWithProxy()
"localhost",
false,
false,
300,
7
);

Expand All @@ -120,6 +121,7 @@ public void ShouldCreateHttpClientHandlerWithoutProxy()
null,
false,
false,
300,
0
);

Expand Down
8 changes: 8 additions & 0 deletions Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
string defProxyPort = "1234";
string defNonProxyHosts = "localhost";

string defRetryTimeout = "300";
string defMaxHttpRetries = "7";
string defIncludeRetryReason = "true";
string defDisableQueryContextCache = "false";
Expand All @@ -82,6 +83,7 @@ public static IEnumerable<TestCase> 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 }
Expand All @@ -107,6 +109,7 @@ public static IEnumerable<TestCase> 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 }
Expand Down Expand Up @@ -135,6 +138,7 @@ public static IEnumerable<TestCase> 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 }
Expand Down Expand Up @@ -165,6 +169,7 @@ public static IEnumerable<TestCase> 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 }
Expand Down Expand Up @@ -193,6 +198,7 @@ public static IEnumerable<TestCase> 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 },
Expand Down Expand Up @@ -220,6 +226,7 @@ public static IEnumerable<TestCase> 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 }
Expand All @@ -245,6 +252,7 @@ public static IEnumerable<TestCase> 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" }
Expand Down
Loading

0 comments on commit 8007e5e

Please sign in to comment.