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 902611 new session pool lifecycle #789

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a65c00f
SNOW-902611 move control over creation of session to session pool
sfc-gh-mhofman Oct 5, 2023
3642b96
SNOW-902611 bugfix on failed session open
sfc-gh-mhofman Oct 6, 2023
52d4038
SNOW-902611 bugfixes for open failures
sfc-gh-mhofman Oct 6, 2023
f53ae8b
SNOW-902611 bugfixe for async open failure
sfc-gh-mhofman Oct 6, 2023
e1a14f5
SNOW-902611 bugfixe for async close failures
sfc-gh-mhofman Oct 6, 2023
4825671
enabled pooling to check failures
sfc-gh-mhofman Oct 6, 2023
09c7b3d
disable pooling to check failures
sfc-gh-mhofman Oct 6, 2023
1616fed
SNOW-902611 refactor improvements
sfc-gh-mhofman Oct 9, 2023
2b09dea
SNOW-902611 teardown of pool settings changes in tests
sfc-gh-mhofman Oct 9, 2023
0521968
SNOW-902611 removed unnecessary catch
sfc-gh-mhofman Oct 9, 2023
8065f28
SNOW-902611 refactor lifecycle of SessionPool, remove its singleton, …
sfc-gh-mhofman Oct 9, 2023
5e490a3
SNOW-902611 refactor lifecycle of SessionPool, remove its singleton, …
sfc-gh-mhofman Oct 9, 2023
3ce270a
SNOW-902611 move control over creation of session to session pool (#783)
sfc-gh-mhofman Oct 10, 2023
ba4d241
Merge fix
sfc-gh-mhofman Oct 10, 2023
7e0d05d
SNOW-902611 bugfixes for open failures
sfc-gh-mhofman Oct 6, 2023
acd2282
SNOW-902611 bugfixe for async close failures
sfc-gh-mhofman Oct 6, 2023
1ca59e1
disable pooling to check failures
sfc-gh-mhofman Oct 6, 2023
cca78fa
SNOW-902611 teardown of pool settings changes in tests
sfc-gh-mhofman Oct 9, 2023
7f66bb2
SNOW-902611 removed unnecessary catch
sfc-gh-mhofman Oct 9, 2023
c82c428
Test fixes after rebase
sfc-gh-mhofman Oct 10, 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
10 changes: 5 additions & 5 deletions Snowflake.Data.Tests/App.config
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Copyright (c) 2012-2017 Snowflake Computing Inc. All rights reserved.
<configSections>
<section name="log4net" type="log4net.Config.Log4NetConfigurationSectionHandler, log4net" />
</configSections>

<log4net>
<appender name="RollingFileAppender" type="log4net.Appender.RollingFileAppender">
<file type="log4net.Util.PatternString" value="test_%property{framework}.log" />
Expand All @@ -29,7 +29,7 @@ Copyright (c) 2012-2017 Snowflake Computing Inc. All rights reserved.
<root>
<level value="ALL" />
<appender-ref ref="RollingFileAppender" />
</root>
</root>
<root>
<level value="WARN" />
<appender-ref ref="ConsoleAppender" />
Expand All @@ -39,12 +39,12 @@ Copyright (c) 2012-2017 Snowflake Computing Inc. All rights reserved.
<!-- used in SFDbFactoryIT.cs to test registering DbProviderFactoryClass -->
<system.data>
<DbProviderFactories>
<add name="Snowflake" invariant="Snowflake.Data"
type="Snowflake.Data.Client.SnowflakeDbFactory, Snowflake.Data, Culture=neutral, PublicKeyToken=null"
<add name="Snowflake" invariant="Snowflake.Data"
type="Snowflake.Data.Client.SnowflakeDbFactory, Snowflake.Data, Culture=neutral, PublicKeyToken=null"
description="Snowflake Provider" />
</DbProviderFactories>
</system.data>

<!--
=========== Enable Network debug log ===============
<system.diagnostics>
Expand Down
24 changes: 24 additions & 0 deletions Snowflake.Data.Tests/IntegrationTests/PoolConfigRestorer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using Snowflake.Data.Client;

namespace Snowflake.Data.Tests.IntegrationTests
{
class PoolConfigRestorer {
private readonly bool _pooling;
private readonly long _timeout;
private readonly int _maxPoolSize;

public PoolConfigRestorer()
{
_maxPoolSize = SnowflakeDbConnectionPool.GetMaxPoolSize();
_timeout = SnowflakeDbConnectionPool.GetTimeout();
_pooling = SnowflakeDbConnectionPool.GetPooling();
}

public void Reset()
{
SnowflakeDbConnectionPool.SetMaxPoolSize(_maxPoolSize);
SnowflakeDbConnectionPool.SetTimeout(_timeout);
SnowflakeDbConnectionPool.SetPooling(_pooling);
}
}
}
19 changes: 15 additions & 4 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,29 @@ namespace Snowflake.Data.Tests.IntegrationTests
using Snowflake.Data.Tests.Mock;
using System.Runtime.InteropServices;

[TestFixture]
[TestFixture, NonParallelizable]
class SFConnectionIT : SFBaseTest
{
private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<SFConnectionIT>();
private static readonly PoolConfigRestorer s_previousPoolConfig = new PoolConfigRestorer();

[SetUp]
public void BeforeTest()
{
s_previousPoolConfig.Reset();
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()
{
s_previousPoolConfig.Reset();
}
[Test]
public void TestBasicConnection()
{
using (IDbConnection conn = new SnowflakeDbConnection())
{
SnowflakeDbConnectionPool.SetPooling(false);
conn.ConnectionString = ConnectionString;
conn.Open();
Assert.AreEqual(ConnectionState.Open, conn.State);
Expand Down Expand Up @@ -137,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}");
Expand Down Expand Up @@ -1889,9 +1900,9 @@ public void TestCloseAsync()
[Test, NonParallelizable]
public void TestCloseAsyncFailure()
{
SnowflakeDbConnectionPool.SetPooling(false);
using (var conn = new MockSnowflakeDbConnection(new MockCloseSessionException()))
{
SnowflakeDbConnectionPool.SetPooling(false);
conn.ConnectionString = ConnectionString;
Assert.AreEqual(conn.State, ConnectionState.Closed);
Task task = null;
Expand Down
33 changes: 6 additions & 27 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,24 @@ namespace Snowflake.Data.Tests.IntegrationTests
using Snowflake.Data.Tests.Mock;
using System.Data.Common;
using Moq;

class PoolConfig {
private readonly bool _pooling;
private readonly long _timeout;
private readonly int _maxPoolSize;

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 SFLogger logger = SFLoggerFactory.GetLogger<SFConnectionPoolT>();
private static readonly PoolConfig previousPoolConfig = new PoolConfig();
private static readonly PoolConfigRestorer s_previousPoolConfig = new PoolConfigRestorer();

[SetUp]
public void BeforeTest()
{
previousPoolConfig.Reset();
s_previousPoolConfig.Reset();
SnowflakeDbConnectionPool.SetPooling(true);
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()
{
previousPoolConfig.Reset();
s_previousPoolConfig.Reset();
}

[OneTimeTearDown]
Expand Down Expand Up @@ -435,20 +414,20 @@ public void TestConnectionPoolTurnOff()
class SFConnectionPoolITAsync : SFBaseTestAsync
{
private static SFLogger logger = SFLoggerFactory.GetLogger<SFConnectionPoolITAsync>();
private static readonly PoolConfig previousPoolConfig = new PoolConfig();
private static readonly PoolConfigRestorer s_previousPoolConfigRestorer = new PoolConfigRestorer();

[SetUp]
public void BeforeTest()
{
previousPoolConfig.Reset();
s_previousPoolConfigRestorer.Reset();
SnowflakeDbConnectionPool.SetPooling(true);
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()
{
previousPoolConfig.Reset();
s_previousPoolConfigRestorer.Reset();
}

[OneTimeTearDown]
Expand Down
3 changes: 0 additions & 3 deletions Snowflake.Data.Tests/IntegrationTests/SFDbCommandIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
2 changes: 1 addition & 1 deletion Snowflake.Data.Tests/Mock/MockRetryUntilRestTimeout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected override async Task<HttpResponseMessage> 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));
}
}
Expand Down
5 changes: 5 additions & 0 deletions Snowflake.Data.Tests/Mock/MockSnowflakeDbConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,10 @@ private void OnSessionEstablished()
{
_connectionState = ConnectionState.Open;
}

protected override bool CanReuseSession(TransactionRollbackStatus transactionRollbackStatus)
{
return false;
}
}
}
77 changes: 27 additions & 50 deletions Snowflake.Data/Client/SnowflakeDbConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -246,39 +246,28 @@ public override void Open()
logger.Debug($"Open with a connection already opened: {_connectionState}");
return;
}
SfSession = SnowflakeDbConnectionPool.GetSession(this.ConnectionString);
if (SfSession != null)
try
{
OnSessionConnecting();
SfSession = SnowflakeDbConnectionPool.GetSession(ConnectionString, Password);
if (SfSession == null)
throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Could not open session");
logger.Debug($"Connection open with pooled session: {SfSession.sessionId}");
OnSessionEstablished();
}
else
catch (Exception e)
{
SetSession();
try
{
SfSession.Open();
}
catch (Exception e)
{
// Otherwise when Dispose() is called, the close request would timeout.
_connectionState = ConnectionState.Closed;
logger.Error("Unable to connect", e);
if (!(e.GetType() == typeof(SnowflakeDbException)))
{
throw
new SnowflakeDbException(
e,
SnowflakeDbException.CONNECTION_FAILURE_SSTATE,
SFError.INTERNAL_ERROR,
"Unable to connect. " + e.Message);
}
else
{
throw;
}
}
// Otherwise when Dispose() is called, the close request would timeout.
_connectionState = ConnectionState.Closed;
logger.Error("Unable to connect: ", e);
if (e is SnowflakeDbException)
throw;
throw new SnowflakeDbException(
e,
SnowflakeDbException.CONNECTION_FAILURE_SSTATE,
SFError.INTERNAL_ERROR,
"Unable to connect. " + e.Message);
}
OnSessionEstablished();
}

public override Task OpenAsync(CancellationToken cancellationToken)
Expand All @@ -289,19 +278,11 @@ public override Task OpenAsync(CancellationToken cancellationToken)
logger.Debug($"Open with a connection already opened: {_connectionState}");
return Task.CompletedTask;
}
SfSession = SnowflakeDbConnectionPool.GetSession(this.ConnectionString);
if (SfSession != null)
{
logger.Debug($"Connection open with pooled session: {SfSession.sessionId}");
OnSessionEstablished();
return Task.CompletedTask;
}

registerConnectionCancellationCallback(cancellationToken);
SetSession();

return SfSession.OpenAsync(cancellationToken).ContinueWith(
previousTask =>
OnSessionConnecting();
return SnowflakeDbConnectionPool
.GetSessionAsync(ConnectionString, Password, cancellationToken)
.ContinueWith(previousTask =>
{
if (previousTask.IsFaulted)
{
Expand All @@ -322,8 +303,9 @@ public override Task OpenAsync(CancellationToken cancellationToken)
}
else
{
logger.Debug("All good");
// Only continue if the session was opened successfully
SfSession = previousTask.Result;
logger.Debug($"Connection open with pooled session: {SfSession.sessionId}");
OnSessionEstablished();
}
},
Expand All @@ -345,19 +327,14 @@ public void SetArrayBindStageCreated()
_isArrayBindStageCreated = true;
}

/// <summary>
/// Create a new SFsession with the connection string settings.
/// </summary>
/// <exception cref="SnowflakeDbException">If the connection string can't be processed</exception>
private void SetSession()
private void OnSessionConnecting()
{
SfSession = new SFSession(ConnectionString, Password);
_connectionTimeout = (int)SfSession.connectionTimeout.TotalSeconds;
_connectionState = ConnectionState.Connecting;
}

private void OnSessionEstablished()
{
_connectionTimeout = (int)SfSession.connectionTimeout.TotalSeconds;
_connectionState = ConnectionState.Open;
}

Expand Down
Loading