Skip to content

Commit

Permalink
SNOW-984600 Avoid closing expired sessions synchronously with obtaine… (
Browse files Browse the repository at this point in the history
#841)

…d lock (#830)

### Description
Avoid closing expired sessions synchronously with obtained lock

### 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`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
  • Loading branch information
sfc-gh-knozderko authored Dec 20, 2023
1 parent aaa32fd commit 5e12bbb
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 10 deletions.
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 @@ private void CleanExpiredSessions()
{
if (item.IsExpired(_timeout, timeNow))
{
Task.Run(() => item.close());
_sessions.Remove(item);
item.close();
}
}
}
Expand Down Expand Up @@ -96,7 +96,7 @@ private SFSession GetIdleSession(string connStr)
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 @@ internal void ClearAllPools()
{
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

0 comments on commit 5e12bbb

Please sign in to comment.