From e6c3914b8815463fcd8421bcc61c6ef8b7a9e712 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 24 Jun 2024 16:30:09 -0700 Subject: [PATCH 1/6] Class skeleton --- .../src/Microsoft.Data.SqlClient.csproj | 4 ++ .../Data/SqlClientX/PoolingDataSource.cs | 54 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index 55170b07e7..7e6cb866e7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -920,6 +920,7 @@ + @@ -967,6 +968,9 @@ + + + diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs new file mode 100644 index 0000000000..d81542e289 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs @@ -0,0 +1,54 @@ +// 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. + +#if NET8_0_OR_GREATER + +using System.Collections.Concurrent; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient; + +namespace Microsoft.Data.SqlClientX +{ + internal sealed class PoolingDataSource : SqlDataSource + { + private SemaphoreSlim poolSemaphore; + private ConcurrentQueue _sqlConnectors; + + private DbConnectionPoolGroupOptions _connectionPoolGroupOptions; + + internal int MinPoolSize => _connectionPoolGroupOptions.MinPoolSize; + internal int MaxPoolSize => _connectionPoolGroupOptions.MaxPoolSize; + + //TODO: support auth contexts and provider info + PoolingDataSource(string connectionString, SqlCredential credential, DbConnectionPoolGroupOptions options) : base(connectionString, credential) + { + _connectionPoolGroupOptions = options; + poolSemaphore = new SemaphoreSlim(MaxPoolSize); + } + + internal override ValueTask GetInternalConnection(bool async, CancellationToken cancellationToken) + { + try + { + if (async) + { + poolSemaphore.WaitAsync(); + } else + { + poolSemaphore.Wait(); + } + + + } + catch + { + poolSemaphore.Release(1); + } + } + } +} + +#endif \ No newline at end of file From 67507a998d683ef69444c836197bb77673746217 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 1 Jul 2024 11:16:48 -0700 Subject: [PATCH 2/6] Add pooling internals and docs. Add return method to data source interface. --- .../Data/SqlClientX/PoolingDataSource.cs | 64 +++++++++++++------ .../Data/SqlClientX/SqlDataSource.cs | 6 ++ 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs index d81542e289..cb3c5f02f7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs @@ -4,49 +4,73 @@ #if NET8_0_OR_GREATER +using System; using System.Collections.Concurrent; using System.Threading; +using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.Data.ProviderBase; using Microsoft.Data.SqlClient; namespace Microsoft.Data.SqlClientX { + /// + /// A pooling implementation of SqlDataSource. Connections are recycled upon return to be reused by another operation. + /// internal sealed class PoolingDataSource : SqlDataSource { - private SemaphoreSlim poolSemaphore; - private ConcurrentQueue _sqlConnectors; - + private Channel _sqlConnectors; private DbConnectionPoolGroupOptions _connectionPoolGroupOptions; internal int MinPoolSize => _connectionPoolGroupOptions.MinPoolSize; internal int MaxPoolSize => _connectionPoolGroupOptions.MaxPoolSize; + /// + /// Initializes a new PoolingDataSource. + /// + /// The connection string used for connections. + /// The credential used for connections. + /// The options used for this pool. //TODO: support auth contexts and provider info PoolingDataSource(string connectionString, SqlCredential credential, DbConnectionPoolGroupOptions options) : base(connectionString, credential) { _connectionPoolGroupOptions = options; - poolSemaphore = new SemaphoreSlim(MaxPoolSize); + //TODO: other construction } + /// internal override ValueTask GetInternalConnection(bool async, CancellationToken cancellationToken) { - try - { - if (async) - { - poolSemaphore.WaitAsync(); - } else - { - poolSemaphore.Wait(); - } - - - } - catch - { - poolSemaphore.Release(1); - } + throw new NotImplementedException(); + } + + internal ValueTask OpenNewInternalConnection(bool async, CancellationToken cancellationToken) + { + + } + + /// + internal override void ReturnInternalConnection(SqlConnector connection) + { + throw new NotImplementedException(); + } + + /// + /// Closes extra idle connections. + /// + /// + internal void PruneIdleConnections() + { + throw new NotImplementedException(); + } + + /// + /// Warms up the pool to bring it up to min pool size. + /// + /// + internal void WarmUp() + { + throw new NotImplementedException(); } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs index 8aa2181557..564bfa6837 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs @@ -51,6 +51,12 @@ protected override SqlConnectionX CreateDbConnection() /// The token used to cancel an ongoing asynchronous call. /// internal abstract ValueTask GetInternalConnection(bool async, CancellationToken cancellationToken); + + /// + /// Returns a SqlConnector to the data source for recycling or finalization. + /// + /// The connection returned to the data source. + internal abstract void ReturnInternalConnection(SqlConnector connection) } } From fc938683e7dc089bb77944e15da45d8e7eb3ca03 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 1 Jul 2024 11:41:11 -0700 Subject: [PATCH 3/6] Add async param to data source return. Add open internal method to data source. --- .../Data/SqlClientX/PoolingDataSource.cs | 21 +++++++++------- .../Data/SqlClientX/SqlDataSource.cs | 15 ++++++++++-- .../Data/SqlClientX/UnpooledDataSource.cs | 24 ++++++++++++------- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs index cb3c5f02f7..225b807a36 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/PoolingDataSource.cs @@ -6,6 +6,8 @@ using System; using System.Collections.Concurrent; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Channels; using System.Threading.Tasks; @@ -19,7 +21,6 @@ namespace Microsoft.Data.SqlClientX /// internal sealed class PoolingDataSource : SqlDataSource { - private Channel _sqlConnectors; private DbConnectionPoolGroupOptions _connectionPoolGroupOptions; internal int MinPoolSize => _connectionPoolGroupOptions.MinPoolSize; @@ -28,29 +29,31 @@ internal sealed class PoolingDataSource : SqlDataSource /// /// Initializes a new PoolingDataSource. /// - /// The connection string used for connections. - /// The credential used for connections. - /// The options used for this pool. //TODO: support auth contexts and provider info - PoolingDataSource(string connectionString, SqlCredential credential, DbConnectionPoolGroupOptions options) : base(connectionString, credential) + PoolingDataSource(SqlConnectionStringBuilder connectionStringBuilder, + SqlCredential credential, + RemoteCertificateValidationCallback userCertificateValidationCallback, + Action clientCertificatesCallback, + DbConnectionPoolGroupOptions options) : base(connectionStringBuilder, credential, userCertificateValidationCallback, clientCertificatesCallback) { _connectionPoolGroupOptions = options; //TODO: other construction } /// - internal override ValueTask GetInternalConnection(bool async, CancellationToken cancellationToken) + internal override ValueTask GetInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) { throw new NotImplementedException(); } - internal ValueTask OpenNewInternalConnection(bool async, CancellationToken cancellationToken) + /// + internal override ValueTask OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) { - + throw new NotImplementedException(); } /// - internal override void ReturnInternalConnection(SqlConnector connection) + internal override ValueTask ReturnInternalConnection(bool async, SqlConnector connection) { throw new NotImplementedException(); } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs index 01e9c8ca0f..17acff6897 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlDataSource.cs @@ -105,12 +105,23 @@ internal SqlDataSource( /// Cancels an outstanding asynchronous operation. /// internal abstract ValueTask GetInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken); - + /// /// Returns a SqlConnector to the data source for recycling or finalization. /// + /// Whether this method should be run asynchronously. /// The connection returned to the data source. - internal abstract void ReturnInternalConnection(SqlConnector connection) + internal abstract ValueTask ReturnInternalConnection(bool async, SqlConnector connection); + + /// + /// Opens a new SqlConnector. + /// + /// The SqlConnectionX object that will exclusively own and use this connector. + /// The connection timeout for this operation. + /// Whether this method should be run asynchronously. + /// Cancels an outstanding asynchronous operation. + /// + internal abstract ValueTask OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken); } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs index aa75a390dc..8e8f27b19a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs @@ -38,20 +38,26 @@ internal UnpooledDataSource( { } - /// - /// Creates and opens a new SqlConnector. - /// - /// The SqlConnectionX object that will exclusively own and use this connector. - /// The connection timeout for this operation. - /// Whether this method should be run asynchronously. - /// Cancels an outstanding asynchronous operation. - /// - internal override async ValueTask GetInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) + + /// + internal override ValueTask GetInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) + { + return OpenNewInternalConnection(owningConnection, timeout, async, cancellationToken); + } + + /// + internal override async ValueTask OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) { var connector = new SqlConnector(owningConnection); await connector.Open(timeout, async, cancellationToken).ConfigureAwait(false); return connector; } + + /// + internal override async ValueTask ReturnInternalConnection(bool async, SqlConnector connection) + { + await connection.Close(async).ConfigureAwait(false); + } } } From d9d77f27f6f255cffb118305a0f17fe247a5e4cb Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 1 Jul 2024 11:42:26 -0700 Subject: [PATCH 4/6] Undo cs proj change. --- .../netcore/src/Microsoft.Data.SqlClient.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index 10b4abad6a..7f8e6426c1 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -979,9 +979,6 @@ - - - From d3ac251d2ce0bf06f95b8980e370fb17cc812a2f Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 1 Jul 2024 12:04:54 -0700 Subject: [PATCH 5/6] Adjust return flow. Make connector methods internal. --- .../Data/SqlClientX/SqlConnectionX.cs | 21 +++++++++----- .../Microsoft/Data/SqlClientX/SqlConnector.cs | 29 +++++++++++++------ .../Data/SqlClientX/UnpooledDataSource.cs | 2 +- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs index 5b4e1cbede..47d3be1e54 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs @@ -30,7 +30,7 @@ internal sealed class SqlConnectionX : DbConnection, ICloneable { private SqlCredential? _credential; private SqlDataSource? _dataSource; - private SqlConnector? _connection; + private SqlConnector? _internalConnection; //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; @@ -184,13 +184,20 @@ internal Task Close(bool async) async Task CloseAsync(bool async) { - Debug.Assert(_connection != null); - var internalConnection = _connection; + Debug.Assert(_internalConnection != null); + Debug.Assert(_dataSource != null); - //TODO: cancel outstanding operations on connection before close - await internalConnection.Close(async).ConfigureAwait(false); + var internalConnection = _internalConnection; - //TODO: remove the internal connection's reference to this wrapper connection + if (internalConnection != null) + { + //TODO: cancel outstanding operations on connection before close + //TODO: if pooling, reset the connector + + await internalConnection.Return(async).ConfigureAwait(false); + internalConnection.OwningConnection = null; + _internalConnection = null; + } _connectionState = ConnectionState.Closed; } @@ -245,7 +252,7 @@ internal async Task Open(bool async, CancellationToken cancellationToken) throw ADP.NoConnectionString(); } - _connection = await _dataSource.GetInternalConnection(this, TimeSpan.FromSeconds(ConnectionTimeout), async, cancellationToken).ConfigureAwait(false); + _internalConnection = await _dataSource.GetInternalConnection(this, TimeSpan.FromSeconds(ConnectionTimeout), async, cancellationToken).ConfigureAwait(false); } #endregion diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnector.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnector.cs index 2a11d137e4..e9a8d391a3 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnector.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnector.cs @@ -10,6 +10,8 @@ using System.Threading; using System.Threading.Tasks; +#nullable enable + namespace Microsoft.Data.SqlClientX { /// @@ -17,25 +19,27 @@ namespace Microsoft.Data.SqlClientX /// internal class SqlConnector { - private WeakReference _owningConnection; + internal SqlConnectionX? OwningConnection { get; set; } + /// - /// The datasource accessed by this connector. + /// The data source that generated this connector. /// - public string DataSource => throw new NotImplementedException(); + internal SqlDataSource DataSource { get; } /// /// The server version this connector is connected to. /// - public string ServerVersion => throw new NotImplementedException(); + internal string ServerVersion => throw new NotImplementedException(); /// /// Represents the current state of this connection. /// - public ConnectionState State => throw new NotImplementedException(); + internal ConnectionState State => throw new NotImplementedException(); - internal SqlConnector(SqlConnectionX owningConnection) + internal SqlConnector(SqlConnectionX owningConnection, SqlDataSource dataSource) { - _owningConnection = new WeakReference(owningConnection); + OwningConnection = owningConnection; + DataSource = dataSource; } /// @@ -44,7 +48,7 @@ internal SqlConnector(SqlConnectionX owningConnection) /// Whether this method should run asynchronously. /// A Task indicating the result of the operation. /// - public Task Close(bool async) + internal ValueTask Close(bool async) { throw new NotImplementedException(); } @@ -57,10 +61,17 @@ public Task Close(bool async) /// The token used to cancel an ongoing asynchronous call. /// A Task indicating the result of the operation. /// - public Task Open(TimeSpan timeout, bool async, CancellationToken cancellationToken) + internal ValueTask Open(TimeSpan timeout, bool async, CancellationToken cancellationToken) { throw new NotImplementedException(); } + + /// + /// Returns this connection to the data source that generated it. + /// + /// Whether this method should run asynchronously. + /// + internal ValueTask Return(bool async) => DataSource.ReturnInternalConnection(async, this); } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs index 8e8f27b19a..993045e33c 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/UnpooledDataSource.cs @@ -48,7 +48,7 @@ internal override ValueTask GetInternalConnection(SqlConnectionX o /// internal override async ValueTask OpenNewInternalConnection(SqlConnectionX owningConnection, TimeSpan timeout, bool async, CancellationToken cancellationToken) { - var connector = new SqlConnector(owningConnection); + var connector = new SqlConnector(owningConnection, this); await connector.Open(timeout, async, cancellationToken).ConfigureAwait(false); return connector; } From f7b118591db74dc6e05d51282699c444ec6c9e7f Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Mon, 1 Jul 2024 13:06:25 -0700 Subject: [PATCH 6/6] Change order of return and owning connection reference drop. --- .../netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs index 47d3be1e54..8d502b2ced 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClientX/SqlConnectionX.cs @@ -194,8 +194,8 @@ async Task CloseAsync(bool async) //TODO: cancel outstanding operations on connection before close //TODO: if pooling, reset the connector - await internalConnection.Return(async).ConfigureAwait(false); internalConnection.OwningConnection = null; + await internalConnection.Return(async).ConfigureAwait(false); _internalConnection = null; }