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

Core pooling logic #2686

Merged
merged 28 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6abf233
Pull over rough outline from npgsql
mdaigle Jul 16, 2024
a470af5
Pull over pool tests. Does not compile yet.
mdaigle Jul 16, 2024
e544f28
Set up test structure. Refactors for visibility for testing.
mdaigle Jul 17, 2024
f228114
Implement disposal. Set ConnectionString property.
mdaigle Jul 17, 2024
1cd0a58
Disable pool blocking for tests. Pull over internal connection open l…
mdaigle Jul 17, 2024
5bd5538
Implement return in pool. Clean up return flow in connection objects.
mdaigle Jul 17, 2024
8124da3
Fix CreateConnection method in SqlDataSource.
mdaigle Jul 18, 2024
5cda5d1
Adjust timeout exception.
mdaigle Jul 18, 2024
16d196b
Add data source statistics. Fix connection sync dispose to invoke clo…
mdaigle Jul 18, 2024
7ad8c2b
Enable exercise test.
mdaigle Jul 18, 2024
3f7ce7e
Merge branch 'feat/sqlclientx' into pooling-internals
mdaigle Jul 18, 2024
47f8431
Add license statements.
mdaigle Jul 19, 2024
9c16cdb
Review changes.
mdaigle Jul 19, 2024
c5a557a
Merge branch 'pooling-internals' of github.com:mdaigle/SqlClient into…
mdaigle Jul 19, 2024
c354c1e
Add comments and adjust method visibility.
mdaigle Jul 19, 2024
11d63ab
Remove unused using statements.
mdaigle Jul 19, 2024
0d9b53a
Adding braces.
mdaigle Jul 22, 2024
394db3c
Clean up loop syntax.
mdaigle Jul 22, 2024
e8896a2
Address review comments.
mdaigle Jul 22, 2024
937a0c8
Add sleep to exercise test.
mdaigle Jul 23, 2024
fdccf10
Review changes.
mdaigle Jul 23, 2024
d292818
Adjust signature.
mdaigle Jul 23, 2024
264af95
Merge branch 'feat/sqlclientx' into pooling-internals
mdaigle Jul 23, 2024
5273804
Clean up commented code.
mdaigle Jul 24, 2024
f6da3f3
Merge branch 'pooling-internals' of github.com:mdaigle/SqlClient into…
mdaigle Jul 24, 2024
c8ba462
Shift wait to connector.
mdaigle Jul 24, 2024
fdd3abc
Use SqlConnectionString instead of builder.
mdaigle Jul 24, 2024
75e5b3b
Add attribution statement.
mdaigle Jul 25, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@
<Compile Include="Microsoft\Data\SqlClientX\SqlConnector.cs" />
<Compile Include="Microsoft\Data\SqlClientX\SqlDataSource.cs" />
<Compile Include="Microsoft\Data\SqlClientX\SqlDataSourceBuilder.cs" />
<Compile Include="Microsoft\Data\SqlClientX\ThrowHelper.cs" />
<Compile Include="Microsoft\Data\SqlClientX\UnpooledDataSource.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<EmbeddedResource Include="$(CommonSourceRoot)Resources\Strings.resx">
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ internal sealed class SqlConnectionX : DbConnection, ICloneable
private SqlDataSource? _dataSource;
private SqlConnector? _internalConnection;

bool _disposed;

internal SqlConnector? InternalConnection { get => _internalConnection; }
mdaigle marked this conversation as resolved.
Show resolved Hide resolved

//TODO: Investigate if we can just use dataSource.ConnectionString. Do this when this class can resolve its own data source.
private string _connectionString = string.Empty;
internal SqlConnectionStringBuilder Settings { get; private set; } = DefaultSettings;
static readonly SqlConnectionStringBuilder DefaultSettings = new();
mdaigle marked this conversation as resolved.
Show resolved Hide resolved

private ConnectionState _connectionState = ConnectionState.Closed;

#region constructors
Expand All @@ -49,6 +56,7 @@ public SqlConnectionX()
/// </summary>
internal SqlConnectionX(string connectionString) : this()
{
_connectionState = ConnectionState.Connecting;
_connectionString = connectionString;
}

Expand Down Expand Up @@ -97,8 +105,7 @@ public override string Database
=> throw new NotImplementedException();

/// <inheritdoc/>
public override int ConnectionTimeout
=> throw new NotImplementedException();
public override int ConnectionTimeout => Settings.ConnectTimeout;

/// <summary>
/// Gets or sets the connection string used to connect to the database.
Expand Down Expand Up @@ -182,7 +189,7 @@ internal Task Close(bool async)

return CloseAsync(async);

async Task CloseAsync(bool async)
Task CloseAsync(bool async)
{
Debug.Assert(_internalConnection != null);
Debug.Assert(_dataSource != null);
Expand All @@ -195,20 +202,40 @@ async Task CloseAsync(bool async)
//TODO: if pooling, reset the connector

internalConnection.OwningConnection = null;
await internalConnection.Return(async).ConfigureAwait(false);
internalConnection.Return();
_internalConnection = null;
}

_connectionState = ConnectionState.Closed;
return Task.CompletedTask;
}
}

#endregion

/// <summary>
/// Releases all resources used by the <see cref="SqlConnectionX"/>.
/// </summary>
/// <param name="disposing"><see langword="true"/> when called from <see cref="Dispose"/>;
/// <see langword="false"/> when being called from the finalizer.</param>
protected override void Dispose(bool disposing)
{
if (_disposed)
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
return;
if (disposing)
Close();
_disposed = true;
}

/// <inheritdoc/>
public override ValueTask DisposeAsync()
=> throw new NotImplementedException();
public override async ValueTask DisposeAsync()
{
if (_disposed)
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
return;

await CloseAsync().ConfigureAwait(false);
_disposed = true;
}
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
/// <inheritdoc/>
public override void EnlistTransaction(Transaction? transaction)
=> throw new NotImplementedException();
Expand Down Expand Up @@ -253,6 +280,7 @@ internal async Task Open(bool async, CancellationToken cancellationToken)
}

_internalConnection = await _dataSource.GetInternalConnection(this, TimeSpan.FromSeconds(ConnectionTimeout), async, cancellationToken).ConfigureAwait(false);
_connectionState = ConnectionState.Open;
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

using System;
using System.Data;
using System.Data.Common;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -34,21 +33,31 @@ internal class SqlConnector
/// <summary>
/// Represents the current state of this connection.
/// </summary>
internal ConnectionState State => throw new NotImplementedException();
/// TODO: set and change state appropriately
internal ConnectionState State = ConnectionState.Open;

internal bool IsOpen => State == ConnectionState.Open;
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
internal bool IsClosed => State == ConnectionState.Closed;
internal bool IsBroken => State == ConnectionState.Broken;

//TODO: set this based on login info
internal int ServerProcessId { get; private set; }

private static int SpoofedServerProcessId = 1;

internal SqlConnector(SqlConnectionX owningConnection, SqlDataSource dataSource)
{
OwningConnection = owningConnection;
DataSource = dataSource;
ServerProcessId = Interlocked.Increment(ref SpoofedServerProcessId);
}

/// <summary>
/// Closes this connection. If this connection is pooled, it is cleaned and returned to the pool.
/// </summary>
/// <param name = "async" > Whether this method should run asynchronously.</param>
/// <returns>A Task indicating the result of the operation.</returns>
/// <exception cref="NotImplementedException"></exception>
internal ValueTask Close(bool async)
internal void Close()
{
throw new NotImplementedException();
}
Expand All @@ -57,21 +66,19 @@ internal ValueTask Close(bool async)
/// Opens this connection.
/// </summary>
/// <param name = "timeout">The connection timeout for this operation.</param>
/// <param name = "async">Whether this method should run asynchronously.</param>
/// <param name = "isAsync">Whether this method should run asynchronously.</param>
/// <param name = "cancellationToken">The token used to cancel an ongoing asynchronous call.</param>
/// <returns>A Task indicating the result of the operation.</returns>
/// <exception cref="NotImplementedException"></exception>
internal ValueTask Open(TimeSpan timeout, bool async, CancellationToken cancellationToken)
internal ValueTask Open(TimeSpan timeout, bool isAsync, CancellationToken cancellationToken)
{
throw new NotImplementedException();
}

/// <summary>
/// Returns this connection to the data source that generated it.
/// </summary>
/// <param name="async">Whether this method should run asynchronously.</param>
/// <returns></returns>
internal ValueTask Return(bool async) => DataSource.ReturnInternalConnection(async, this);
internal void Return() => DataSource.ReturnInternalConnection(this);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,31 @@
// See the LICENSE file in the project root for more information.

#if NET8_0_OR_GREATER
#nullable enable

using System;
using System.Data.Common;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.SqlClient;

namespace Microsoft.Data.SqlClientX
{
//TODO: update this whole class to return SqlConnection after it is changed to wrap SqlConnectionX

/// <summary>
/// Represents a data source that can be used to obtain SqlConnections.
/// SqlDataSource can also create and open SqlConnectors, which are the internal/physical connections wrapped by SqlConnection.
/// </summary>
internal abstract class SqlDataSource : DbDataSource
{
private readonly SqlConnectionStringBuilder _connectionStringBuilder;
private protected volatile int _isDisposed;

internal SqlCredential Credential { get; }

//TODO: return SqlConnection after it is updated to wrap SqlConnectionX
internal abstract (int Total, int Idle, int Busy) Statistics { get; }

/// <summary>
/// Creates a new, unopened SqlConnection.
/// </summary>
Expand All @@ -49,32 +52,31 @@ internal SqlDataSource(
/// <summary>
/// Creates a new <see cref="SqlConnection"/> object.
/// </summary>
public new SqlConnection CreateConnection()
public new SqlConnectionX CreateConnection()
{
throw new NotImplementedException();
// TODO: return (SqlConnection)CreateDbConnection();
return CreateDbConnection();
}

/// <summary>
/// Opens a new <see cref="SqlConnection"/>.
/// </summary>
public new SqlConnection OpenConnection()
public new SqlConnectionX OpenConnection()
{
return (SqlConnection)base.OpenConnection();
return (SqlConnectionX)base.OpenConnection();
}

/// <summary>
/// Asynchronously opens a new <see cref="SqlConnection"/>.
/// </summary>
public new async ValueTask<SqlConnection> OpenConnectionAsync(CancellationToken cancellationToken = default)
public new async ValueTask<SqlConnectionX> OpenConnectionAsync(CancellationToken cancellationToken = default)
{
return (SqlConnection)await base.OpenConnectionAsync(cancellationToken).ConfigureAwait(false);
return (SqlConnectionX)await base.OpenConnectionAsync(cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Creates a new <see cref="SqlCommand"/> object.
/// </summary>
public new SqlCommand CreateCommand(string commandText = null)
public new SqlCommand CreateCommand(string? commandText)
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
{
return (SqlCommand)CreateDbCommand(commandText);
}
Expand All @@ -87,7 +89,6 @@ internal SqlDataSource(
return (SqlBatch)CreateDbBatch();
}

// TODO: make abstract
/// <summary>
/// Returns an opened SqlConnector.
/// </summary>
Expand All @@ -98,12 +99,12 @@ internal SqlDataSource(
/// <returns></returns>
internal abstract ValueTask<SqlConnector> GetInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken);


/// <summary>
/// Returns a SqlConnector to the data source for recycling or finalization.
/// </summary>
/// <param name="async">Whether this method should be run asynchronously.</param>
/// <param name="connection">The connection returned to the data source.</param>
internal abstract ValueTask ReturnInternalConnection(bool async, SqlConnector connection);
/// <param name="connector">The connection returned to the data source.</param>
internal abstract void ReturnInternalConnection(SqlConnector connector);

/// <summary>
/// Opens a new SqlConnector.
Expand All @@ -113,7 +114,14 @@ internal SqlDataSource(
/// <param name="async">Whether this method should be run asynchronously.</param>
/// <param name="cancellationToken">Cancels an outstanding asynchronous operation.</param>
/// <returns></returns>
internal abstract ValueTask<SqlConnector> OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken);
internal abstract ValueTask<SqlConnector?> OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken);

private protected void CheckDisposed()
{
//TODO: implement disposal
if (_isDisposed == 1)
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
ThrowHelper.ThrowObjectDisposedException(GetType().FullName);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ internal sealed class SqlDataSourceBuilder
/// <summary>
/// Constructs a new <see cref="SqlDataSourceBuilder" />, optionally starting out from the given <paramref name="connectionString"/>.
/// </summary>
public SqlDataSourceBuilder(string connectionString = null, SqlCredential credential = null)
public SqlDataSourceBuilder(string connectionString = null, SqlCredential credential = null)
: this(new SqlConnectionStringBuilder(connectionString), credential)
{}

public SqlDataSourceBuilder(SqlConnectionStringBuilder connectionStringBuilder, SqlCredential sqlCredential = null)
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
{
ConnectionStringBuilder = new SqlConnectionStringBuilder(connectionString);
Credential = credential;
ConnectionStringBuilder = connectionStringBuilder;
Credential = sqlCredential;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics.CodeAnalysis;

#nullable enable

namespace Microsoft.Data.SqlClientX
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
{

static class ThrowHelper
mdaigle marked this conversation as resolved.
Show resolved Hide resolved
{
[DoesNotReturn]
internal static void ThrowArgumentOutOfRangeException()
=> throw new ArgumentOutOfRangeException();

[DoesNotReturn]
internal static void ThrowArgumentOutOfRangeException(string paramName, string message)
=> throw new ArgumentOutOfRangeException(paramName, message);

[DoesNotReturn]
internal static void ThrowArgumentOutOfRangeException(string paramName, string message, object argument)
=> throw new ArgumentOutOfRangeException(paramName, string.Format(message, argument));

[DoesNotReturn]
internal static void ThrowInvalidOperationException()
=> throw new InvalidOperationException();

[DoesNotReturn]
internal static void ThrowInvalidOperationException(string message)
=> throw new InvalidOperationException(message);

[DoesNotReturn]
internal static void ThrowInvalidOperationException(string message, object argument)
=> throw new InvalidOperationException(string.Format(message, argument));

[DoesNotReturn]
internal static void ThrowObjectDisposedException(string? objectName)
=> throw new ObjectDisposedException(objectName);

[DoesNotReturn]
internal static void ThrowObjectDisposedException(string objectName, string message)
=> throw new ObjectDisposedException(objectName, message);

[DoesNotReturn]
internal static void ThrowObjectDisposedException(string objectName, Exception? innerException)
=> throw new ObjectDisposedException(objectName, innerException);

[DoesNotReturn]
internal static void ThrowInvalidCastException(string message, object argument)
=> throw new InvalidCastException(string.Format(message, argument));

[DoesNotReturn]
internal static void ThrowInvalidCastException(string message) =>
throw new InvalidCastException(message);

[DoesNotReturn]
internal static void ThrowInvalidCastException_NoValue() =>
throw new InvalidCastException("Field is null.");

[DoesNotReturn]
internal static void ThrowArgumentException(string message)
=> throw new ArgumentException(message);

[DoesNotReturn]
internal static void ThrowArgumentException(string message, string paramName)
=> throw new ArgumentException(message, paramName);

[DoesNotReturn]
internal static void ThrowArgumentNullException(string paramName)
=> throw new ArgumentNullException(paramName);

[DoesNotReturn]
internal static void ThrowArgumentNullException(string message, string paramName)
=> throw new ArgumentNullException(paramName, message);

[DoesNotReturn]
internal static void ThrowIndexOutOfRangeException(string message)
=> throw new IndexOutOfRangeException(message);

[DoesNotReturn]
internal static void ThrowIndexOutOfRangeException(string message, object argument)
=> throw new IndexOutOfRangeException(string.Format(message, argument));

[DoesNotReturn]
internal static void ThrowNotSupportedException(string? message = null)
=> throw new NotSupportedException(message);
}
}
Loading
Loading