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 move control over creation of session to session pool #783

Merged
merged 12 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 9 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
4 changes: 2 additions & 2 deletions Snowflake.Data.Tests/App.config
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Copyright (c) 2012-2017 Snowflake Computing Inc. All rights reserved.
</layout>
<appender name="ConsoleAppender" type="log4net.Appender.ConsoleAppender">
<layout type="log4net.Layout.PatternLayout">
<conversionPattern value="%date [%thread] [%-5level] [%ClassName:%line] - %message%newline" />
<conversionPattern value="%date [%thread] [%-5level] [%classname:%line] - %message%newline" />
</layout>
</appender>
</appender>
Expand All @@ -31,7 +31,7 @@ Copyright (c) 2012-2017 Snowflake Computing Inc. All rights reserved.
<appender-ref ref="RollingFileAppender" />
</root>
<root>
<level value="WARN" />
<level value="ALL" />
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
<appender-ref ref="ConsoleAppender" />
</root>
</log4net>
Expand Down
26 changes: 26 additions & 0 deletions Snowflake.Data.Tests/IntegrationTests/PoolConfigRestorer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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);
}
}


sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
}
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 15 additions & 2 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,24 @@
using Snowflake.Data.Tests.Mock;
using System.Runtime.InteropServices;

[TestFixture]
[TestFixture, NonParallelizable]
sfc-gh-mhofman marked this conversation as resolved.
Show resolved Hide resolved
class SFConnectionIT : SFBaseTest
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<SFConnectionIT>();
private static readonly PoolConfigRestorer s_previousPoolConfig = new PoolConfigRestorer();

[SetUp]
public void BeforeTest()

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 28 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionIT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.
{
s_previousPoolConfig.Reset();
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 35 in Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionIT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.
{
s_previousPoolConfig.Reset();
}
[Test]
public void TestBasicConnection()
{
Expand Down Expand Up @@ -1889,9 +1902,9 @@
[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 @@
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()

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 25 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionPoolT.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.
{
previousPoolConfig.Reset();
s_previousPoolConfig.Reset();
SnowflakeDbConnectionPool.SetPooling(true);
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 33 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionPoolT.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.
{
previousPoolConfig.Reset();
s_previousPoolConfig.Reset();
}

[OneTimeTearDown]
Expand Down Expand Up @@ -435,20 +414,20 @@
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()

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.

Check warning on line 420 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionPoolITAsync.BeforeTest()' hides inherited member 'SFBaseTestAsync.BeforeTest()'. Use the new keyword if hiding was intended.
{
previousPoolConfig.Reset();
s_previousPoolConfigRestorer.Reset();
SnowflakeDbConnectionPool.SetPooling(true);
SnowflakeDbConnectionPool.ClearAllPools();
}

[TearDown]
public void AfterTest()

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.

Check warning on line 428 in Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFConnectionPoolITAsync.AfterTest()' hides inherited member 'SFBaseTestAsync.AfterTest()'. Use the new keyword if hiding was intended.
{
previousPoolConfig.Reset();
s_previousPoolConfigRestorer.Reset();
}

[OneTimeTearDown]
Expand Down
71 changes: 25 additions & 46 deletions Snowflake.Data/Client/SnowflakeDbConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,39 +246,30 @@ 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
// Otherwise when Dispose() is called, the close request would timeout.
_connectionState = ConnectionState.Closed;
logger.Error("Unable to connect: ", e);
if (e is SnowflakeDbException)
{
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;
}
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 +280,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 +305,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 +329,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
16 changes: 12 additions & 4 deletions Snowflake.Data/Client/SnowflakeDbConnectionPool.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using Snowflake.Data.Core;
using System.Security;
using System.Threading;
using System.Threading.Tasks;
using Snowflake.Data.Core;
using Snowflake.Data.Core.Session;
using Snowflake.Data.Log;

Expand All @@ -8,12 +11,17 @@ public class SnowflakeDbConnectionPool
{
private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<SnowflakeDbConnectionPool>();

internal static SFSession GetSession(string connStr)
internal static SFSession GetSession(string connStr, SecureString password)
{
s_logger.Debug("SnowflakeDbConnectionPool::GetSession");
return SessionPoolSingleton.Instance.GetSession(connStr);
return SessionPoolSingleton.Instance.GetSession(connStr, password);
}


internal static Task<SFSession> GetSessionAsync(string connStr, SecureString password, CancellationToken cancellationToken)
{
return SessionPoolSingleton.Instance.GetSessionAsync(connStr, password, cancellationToken);
}

internal static bool AddSession(SFSession session)
{
s_logger.Debug("SnowflakeDbConnectionPool::AddSession");
Expand Down
Loading
Loading