From 8f35e5df4d550c8a0ee6a95ef0079a02a9eac3fd Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Fri, 15 Dec 2023 15:05:01 +0100 Subject: [PATCH] Revert "SNOW-984600 Avoid closing expired sessions synchronously with obtained lock (#830)" This reverts commit 645659a864158b0b65dec2f80a1d0f1db9dc00cc. --- .../IntegrationTests/SFConnectionIT.cs | 8 +++--- .../IntegrationTests/SFConnectionPoolIT.cs | 27 ------------------- Snowflake.Data/Core/Session/SFSession.cs | 8 +++--- Snowflake.Data/Core/Session/SessionPool.cs | 6 ++--- 4 files changed, 10 insertions(+), 39 deletions(-) diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs index c13cd684b..eea852af2 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs @@ -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 3 sec (buffer time) after the defined timeout - Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (timeoutSec + 3) * 1000); + // 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(timeoutSec, conn.ConnectionTimeout); @@ -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 2 sec (buffer time) after the defined timeout - Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (retryTimeout + 2) * 1000); + // 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); diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolIT.cs index 2a403521e..5c4529225 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolIT.cs @@ -12,7 +12,6 @@ using Snowflake.Data.Client; using Snowflake.Data.Log; using NUnit.Framework; -using Snowflake.Data.Core.Session; namespace Snowflake.Data.Tests.IntegrationTests { @@ -396,31 +395,5 @@ 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(); - } } } diff --git a/Snowflake.Data/Core/Session/SFSession.cs b/Snowflake.Data/Core/Session/SFSession.cs index cb9e4f0fe..d153ffff4 100755 --- a/Snowflake.Data/Core/Session/SFSession.cs +++ b/Snowflake.Data/Core/Session/SFSession.cs @@ -242,7 +242,7 @@ internal async Task OpenAsync(CancellationToken cancellationToken) internal void close() { // Nothing to do if the session is not open - if (!IsEstablished()) return; + if (null == sessionToken) return; stopHeartBeatForThisSession(); @@ -274,7 +274,7 @@ internal void close() internal async Task CloseAsync(CancellationToken cancellationToken) { // Nothing to do if the session is not open - if (!IsEstablished()) return; + if (null == sessionToken) return; stopHeartBeatForThisSession(); @@ -303,8 +303,6 @@ internal async Task CloseAsync(CancellationToken cancellationToken) sessionToken = null; } - internal bool IsEstablished() => sessionToken != null; - internal void renewSession() { logger.Info("Renew the session."); @@ -506,7 +504,7 @@ internal void heartbeat() logger.Debug("heartbeat"); bool retry = false; - if (IsEstablished()) + if (sessionToken != null) { do { diff --git a/Snowflake.Data/Core/Session/SessionPool.cs b/Snowflake.Data/Core/Session/SessionPool.cs index 14ad70848..a7eae7726 100644 --- a/Snowflake.Data/Core/Session/SessionPool.cs +++ b/Snowflake.Data/Core/Session/SessionPool.cs @@ -57,8 +57,8 @@ private void CleanExpiredSessions() { if (item.IsExpired(_timeout, timeNow)) { - Task.Run(() => item.close()); _sessions.Remove(item); + item.close(); } } } @@ -96,7 +96,7 @@ private SFSession GetIdleSession(string connStr) long timeNow = DateTimeOffset.UtcNow.ToUnixTimeSeconds(); if (session.IsExpired(_timeout, timeNow)) { - Task.Run(() => session.close()); + session.close(); i--; } else @@ -187,7 +187,7 @@ internal void ClearAllPools() { foreach (SFSession session in _sessions) { - session.close(); // it is left synchronously here because too much async tasks slows down testing + session.close(); } _sessions.Clear(); }