Skip to content

Commit

Permalink
SNOW-902611 move control over creation of session to session pool (#783)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sfc-gh-mhofman committed Oct 10, 2023
1 parent 5e490a3 commit 3ce270a
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 12 deletions.
2 changes: 0 additions & 2 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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}");
Expand Down
21 changes: 18 additions & 3 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionPoolT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -414,7 +429,7 @@ public void TestConnectionPoolTurnOff()
class SFConnectionPoolITAsync : SFBaseTestAsync
{
private static SFLogger logger = SFLoggerFactory.GetLogger<SFConnectionPoolITAsync>();
private static readonly PoolConfigRestorer s_previousPoolConfigRestorer = new PoolConfigRestorer();
private static readonly PoolConfig s_previousPoolConfigRestorer = new PoolConfig();

[SetUp]
public void BeforeTest()
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;
}
}
}
4 changes: 2 additions & 2 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 Down
5 changes: 4 additions & 1 deletion Snowflake.Data/Core/HttpUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,10 @@ protected override async Task<HttpResponseMessage> 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);
Expand Down

0 comments on commit 3ce270a

Please sign in to comment.