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.

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-984600 Avoid closing expired sessions synchronously with obtaine… #841

Merged
merged 1 commit into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1780,8 +1780,8 @@ public void TestAsyncLoginTimeout()

// 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);
// But never more than 3 sec (buffer time) after the defined timeout
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (timeoutSec + 3) * 1000);

Assert.AreEqual(ConnectionState.Closed, conn.State);
Assert.AreEqual(timeoutSec, conn.ConnectionTimeout);
Expand Down Expand Up @@ -1818,8 +1818,8 @@ public void TestAsyncLoginTimeoutWithRetryTimeoutLesserThanConnectionTimeout()

// 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);
// But never more than 2 sec (buffer time) after the defined timeout
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (retryTimeout + 2) * 1000);

Assert.AreEqual(ConnectionState.Closed, conn.State);
Assert.AreEqual(retryTimeout, conn.ConnectionTimeout);
Expand Down
27 changes: 27 additions & 0 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Snowflake.Data.Client;
using Snowflake.Data.Log;
using NUnit.Framework;
using Snowflake.Data.Core.Session;

namespace Snowflake.Data.Tests.IntegrationTests
{
Expand Down Expand Up @@ -395,5 +396,31 @@ public void TestConnectionPoolTurnOff()
SnowflakeDbConnectionPool.SetPooling(false);
//Put a breakpoint at SFSession close function, after connection pool is off, it will send close session request.
}

[Test]
public void TestCloseSessionAfterTimeout()
{
// arrange
const int SessionTimeoutSeconds = 2;
const int TimeForBackgroundSessionCloseMillis = 2000;
SnowflakeDbConnectionPool.SetTimeout(SessionTimeoutSeconds);
var conn1 = new SnowflakeDbConnection(ConnectionString);
conn1.Open();
var session = conn1.SfSession;
conn1.Close();
Assert.IsTrue(session.IsEstablished());
Thread.Sleep(SessionTimeoutSeconds * 1000); // wait until the session is expired
var conn2 = new SnowflakeDbConnection(ConnectionString);

// act
conn2.Open(); // it gets a session from the caching pool firstly closing session of conn1 in background
Thread.Sleep(TimeForBackgroundSessionCloseMillis); // wait for closing expired session

// assert
Assert.IsFalse(session.IsEstablished());

// cleanup
conn2.Close();
}
}
}
8 changes: 5 additions & 3 deletions Snowflake.Data/Core/Session/SFSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ internal async Task OpenAsync(CancellationToken cancellationToken)
internal void close()
{
// Nothing to do if the session is not open
if (null == sessionToken) return;
if (!IsEstablished()) return;

stopHeartBeatForThisSession();

Expand Down Expand Up @@ -274,7 +274,7 @@ internal void close()
internal async Task CloseAsync(CancellationToken cancellationToken)
{
// Nothing to do if the session is not open
if (null == sessionToken) return;
if (!IsEstablished()) return;

stopHeartBeatForThisSession();

Expand Down Expand Up @@ -303,6 +303,8 @@ internal async Task CloseAsync(CancellationToken cancellationToken)
sessionToken = null;
}

internal bool IsEstablished() => sessionToken != null;

internal void renewSession()
{
logger.Info("Renew the session.");
Expand Down Expand Up @@ -504,7 +506,7 @@ internal void heartbeat()
logger.Debug("heartbeat");

bool retry = false;
if (sessionToken != null)
if (IsEstablished())
{
do
{
Expand Down
6 changes: 3 additions & 3 deletions Snowflake.Data/Core/Session/SessionPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
{
if (item.IsExpired(_timeout, timeNow))
{
Task.Run(() => item.close());

Check warning on line 60 in Snowflake.Data/Core/Session/SessionPool.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/SessionPool.cs#L60

Added line #L60 was not covered by tests
_sessions.Remove(item);
item.close();
}
}
}
Expand Down Expand Up @@ -96,7 +96,7 @@
long timeNow = DateTimeOffset.UtcNow.ToUnixTimeSeconds();
if (session.IsExpired(_timeout, timeNow))
{
session.close();
Task.Run(() => session.close());
i--;
}
else
Expand Down Expand Up @@ -187,7 +187,7 @@
{
foreach (SFSession session in _sessions)
{
session.close();
session.close(); // it is left synchronously here because too much async tasks slows down testing
}
_sessions.Clear();
}
Expand Down
Loading