Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-878067: Retry Strategy #803

Merged
merged 24 commits into from
Nov 6, 2023
Merged

SNOW-878067: Retry Strategy #803

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
68e4dc1
SNOW-878067: Add retry strategy for supported endpoints
sfc-gh-ext-simba-lf Oct 26, 2023
b7dc6bd
SNOW-878067: Add tests for retry strategy
sfc-gh-ext-simba-lf Oct 26, 2023
59f1f7e
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Oct 26, 2023
5d7f9e1
SNOW-878067: Fix variable name
sfc-gh-ext-simba-lf Oct 26, 2023
77611e6
SNOW-878067: Increase delta for async timeout test
sfc-gh-ext-simba-lf Oct 26, 2023
9639f43
SNOW-878067: Increase delta for async timeout test
sfc-gh-ext-simba-lf Oct 26, 2023
a441122
SNOW-878067: Increase delta for timeout test
sfc-gh-ext-simba-lf Oct 26, 2023
ddd961e
SNOW-878067: Refactor variable for connection timeout
sfc-gh-ext-simba-lf Oct 27, 2023
a203e19
SNOW-878067: Refactor variable for connection timeout
sfc-gh-ext-simba-lf Oct 27, 2023
949c257
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Oct 27, 2023
da0d1c6
Update CONNECTION_TIMEOUT default value from 120 to 300
sfc-gh-ext-simba-lf Oct 31, 2023
8007e5e
SNOW-878067: Update retry logic
sfc-gh-ext-simba-lf Nov 3, 2023
5b926b7
Update README.md with new parameter "RETRY_TIMEOUT"
sfc-gh-ext-simba-lf Nov 3, 2023
2866dec
SNOW-878067: Modify timeout test cases
sfc-gh-ext-simba-lf Nov 3, 2023
692bb36
Merge branch 'SNOW-878067-Retry-Strategy' of https://github.com/snowf…
sfc-gh-ext-simba-lf Nov 3, 2023
d0a445e
SNOW-878067: Modify timeout test cases
sfc-gh-ext-simba-lf Nov 3, 2023
ce6b2b6
SNOW-878067: Modify timeout test cases
sfc-gh-ext-simba-lf Nov 3, 2023
98f0abe
SNOW-878067: Modify timeout test cases
sfc-gh-ext-simba-lf Nov 3, 2023
b25afec
SNOW-878067: Update comment
sfc-gh-ext-simba-lf Nov 3, 2023
13fa45f
SNOW-878067: Modify timeout test cases
sfc-gh-ext-simba-lf Nov 3, 2023
5e9931e
SNOW-878067: Refactor retry logic
sfc-gh-ext-simba-lf Nov 3, 2023
4cc94f7
SNOW-878067: Set maxHttpRetries to 0 to reach the defined timeout
sfc-gh-ext-simba-lf Nov 3, 2023
ef12cef
SNOW-878067: Remove OS Platform restriction on timeout tests
sfc-gh-ext-simba-lf Nov 3, 2023
4f511f4
SNOW-878067: Refactor setting which timeout to use
sfc-gh-ext-simba-lf Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`. |
Expand Down
94 changes: 51 additions & 43 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
View file
Open in desktop
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(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 @@ -380,7 +380,7 @@ public void TestLoginTimeout()
using (IDbConnection conn = new MockSnowflakeDbConnection())
{
int timeoutSec = 5;
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}",
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};maxHttpRetries=0",
timeoutSec);

conn.ConnectionString = loginTimeOut5sec;
Expand All @@ -391,21 +391,23 @@ public void TestLoginTimeout()
{
conn.Open();
Assert.Fail();

}
catch (AggregateException e)
{
Assert.AreEqual(SFError.REQUEST_TIMEOUT.GetAttribute<SFErrorAttr>().errorCode,
// 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<SFErrorAttr>().errorCode ==
((SnowflakeDbException)e.InnerException).ErrorCode);
}
stopwatch.Stop();
int detla = 10; //in case server time slower.
int delta = 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);
// 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);
}
}
}
Expand All @@ -426,15 +428,20 @@ 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<SFErrorAttr>().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);
}
}

Expand All @@ -446,8 +453,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 @@ -464,10 +471,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);
}
}
}
Expand Down Expand Up @@ -1524,7 +1533,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 @@ -1551,7 +1560,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 @@ -1675,15 +1684,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
Expand All @@ -1704,7 +1712,7 @@ public void TestCancelLoginBeforeTimeout()
}

Assert.AreEqual(ConnectionState.Closed, conn.State);
Assert.AreEqual(0, conn.ConnectionTimeout);
Assert.AreEqual(timeoutSec, conn.ConnectionTimeout);
}
}

Expand All @@ -1716,7 +1724,7 @@ public void TestAsyncLoginTimeout()
using (var conn = new MockSnowflakeDbConnection())
{
int timeoutSec = 5;
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0}",
string loginTimeOut5sec = String.Format(ConnectionString + "connection_timeout={0};maxHttpRetries=0",
timeoutSec);
conn.ConnectionString = loginTimeOut5sec;

Expand All @@ -1735,15 +1743,15 @@ public void TestAsyncLoginTimeout()

}
stopwatch.Stop();
int detla = 10; //in case server time slower.
int delta = 10; // in case server time slower.

// 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);
// 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);

Assert.AreEqual(ConnectionState.Closed, conn.State);
Assert.AreEqual(5, conn.ConnectionTimeout);
Assert.AreEqual(timeoutSec, conn.ConnectionTimeout);
}
}
}
Expand All @@ -1770,15 +1778,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);
}
}

Expand Down
Loading
Loading