From 3ce270a3e70bb55e0913e92bc01a737463e82cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Hofman?= Date: Tue, 10 Oct 2023 16:29:06 +0200 Subject: [PATCH] SNOW-902611 move control over creation of session to session pool (#783) Moving a code responsible for establishing a new session to the session pool class as a prerequisite step to introduction of a new connection pool implementation. Fixed some flaky tests which were unrelated to the pool but were failing when mocked connection (with some intentional throw exception behavior) was put back to the pool by another test. - [x] Code compiles correctly - [x] Code is formatted according to [Coding Conventions](../CodingConventions.md) - [ ] 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 --- .../IntegrationTests/SFConnectionIT.cs | 2 -- .../IntegrationTests/SFConnectionPoolT.cs | 21 ++++++++++++++++--- .../IntegrationTests/SFDbCommandIT.cs | 3 --- .../Mock/MockRetryUntilRestTimeout.cs | 2 +- .../Mock/MockSnowflakeDbConnection.cs | 5 +++++ .../Client/SnowflakeDbConnection.cs | 4 ++-- Snowflake.Data/Core/HttpUtil.cs | 5 ++++- 7 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs index e8b4fe0b8..bd3451a7d 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs @@ -41,7 +41,6 @@ public void TestBasicConnection() { using (IDbConnection conn = new SnowflakeDbConnection()) { - SnowflakeDbConnectionPool.SetPooling(false); conn.ConnectionString = ConnectionString; conn.Open(); Assert.AreEqual(ConnectionState.Open, conn.State); @@ -150,7 +149,6 @@ public void TestIncorrectUserOrPasswordBasicConnection() [TestCase(false)] public void TestConnectionIsNotMarkedAsOpenWhenWasNotCorrectlyOpenedBefore(bool explicitClose) { - SnowflakeDbConnectionPool.SetPooling(true); for (int i = 0; i < 2; ++i) { s_logger.Debug($"Running try #{i}"); diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs index 6f858d7cf..89e297fe9 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs @@ -16,11 +16,26 @@ namespace Snowflake.Data.Tests.IntegrationTests using System.Data.Common; using Moq; + public PoolConfig() + { + _maxPoolSize = SnowflakeDbConnectionPool.GetMaxPoolSize(); + _timeout = SnowflakeDbConnectionPool.GetTimeout(); + _pooling = SnowflakeDbConnectionPool.GetPooling(); + } + + public void Reset() + { + SnowflakeDbConnectionPool.SetMaxPoolSize(_maxPoolSize); + SnowflakeDbConnectionPool.SetTimeout(_timeout); + SnowflakeDbConnectionPool.SetPooling(_pooling); + } + } + [TestFixture, NonParallelizable] class SFConnectionPoolT : SFBaseTest { - private static readonly PoolConfigRestorer s_previousPoolConfig = new PoolConfigRestorer(); - + private static readonly PoolConfig s_previousPoolConfig = new PoolConfig(); + [SetUp] public void BeforeTest() { @@ -414,7 +429,7 @@ public void TestConnectionPoolTurnOff() class SFConnectionPoolITAsync : SFBaseTestAsync { private static SFLogger logger = SFLoggerFactory.GetLogger(); - private static readonly PoolConfigRestorer s_previousPoolConfigRestorer = new PoolConfigRestorer(); + private static readonly PoolConfig s_previousPoolConfigRestorer = new PoolConfig(); [SetUp] public void BeforeTest() diff --git a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs index 98e9af7c1..8532b3bb0 100755 --- a/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs @@ -229,9 +229,6 @@ public void TestSimpleLargeResultSet() } - /* - * Disabled to make sure that configuration changes does not cause problems with appveyor - */ [Test, NonParallelizable] public void TestUseV1ResultParser() { diff --git a/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs b/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs index 8d27a00aa..c9f9a665a 100644 --- a/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs +++ b/Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs @@ -31,7 +31,7 @@ protected override async Task SendAsync(HttpRequestMessage string sid = "") { // Override the http timeout and set to 1ms to force all http request to timeout and retry - message.Properties[BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY] = TimeSpan.FromMilliseconds(1); + message.Properties[BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY] = TimeSpan.FromTicks(0); return await (base.SendAsync(message, restTimeout, externalCancellationToken).ConfigureAwait(false)); } } diff --git a/Snowflake.Data.Tests/Mock/MockSnowflakeDbConnection.cs b/Snowflake.Data.Tests/Mock/MockSnowflakeDbConnection.cs index aa342fe68..0f1758636 100644 --- a/Snowflake.Data.Tests/Mock/MockSnowflakeDbConnection.cs +++ b/Snowflake.Data.Tests/Mock/MockSnowflakeDbConnection.cs @@ -92,5 +92,10 @@ private void OnSessionEstablished() { _connectionState = ConnectionState.Open; } + + protected override bool CanReuseSession(TransactionRollbackStatus transactionRollbackStatus) + { + return false; + } } } diff --git a/Snowflake.Data/Client/SnowflakeDbConnection.cs b/Snowflake.Data/Client/SnowflakeDbConnection.cs index b611589cf..5f66d340c 100755 --- a/Snowflake.Data/Client/SnowflakeDbConnection.cs +++ b/Snowflake.Data/Client/SnowflakeDbConnection.cs @@ -37,7 +37,7 @@ public class SnowflakeDbConnection : DbConnection // Will fix that in a separated PR though as it's a different issue private static Boolean _isArrayBindStageCreated; - private enum TransactionRollbackStatus + protected enum TransactionRollbackStatus { Undefined, // used to indicate ignored transaction status when pool disabled Success, @@ -232,7 +232,7 @@ public Task CloseAsync(CancellationToken cancellationToken) return taskCompletionSource.Task; } - private bool CanReuseSession(TransactionRollbackStatus transactionRollbackStatus) + protected virtual bool CanReuseSession(TransactionRollbackStatus transactionRollbackStatus) { return SnowflakeDbConnectionPool.GetPooling() && transactionRollbackStatus == TransactionRollbackStatus.Success; diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index 0daab0ab6..77d3807ff 100755 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -364,7 +364,10 @@ protected override async Task SendAsync(HttpRequestMessage if (!httpTimeout.Equals(Timeout.InfiniteTimeSpan)) { childCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - childCts.CancelAfter(httpTimeout); + if (httpTimeout.Ticks == 0) + childCts.Cancel(); + else + childCts.CancelAfter(httpTimeout); } response = await base.SendAsync(requestMessage, childCts == null ? cancellationToken : childCts.Token).ConfigureAwait(false);