-
Notifications
You must be signed in to change notification settings - Fork 317
Connection pool transaction stress tests #3805
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
base: main
Are you sure you want to change the base?
Changes from all commits
8c66406
3c74365
39b7851
1f4bc05
fab9a43
cddc9eb
9ba0b9c
e1853da
d4533c6
6cd9e82
d4b1e21
e68ea14
1191ca4
b969832
42c8fbd
1f31bd2
90f2686
861b0e7
017353c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,6 @@ | |
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Azure.Core" /> | ||
| <PackageReference Include="System.Text.Encodings.Web" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the scoop with these package removals? Have the transitive problems been solved somehow?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll clean these up. Visual studio was complaining, so I did this to appease it. I think it was just holding on to stale partial build state. |
||
| <PackageReference Include="Azure.Security.KeyVault.Keys" /> | ||
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" /> | ||
| </ItemGroup> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,9 +42,6 @@ | |
| <PackageReference Include="Microsoft.SqlServer.Server" /> | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" /> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
|
|
||
| <!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. --> | ||
| <PackageReference Include="System.Text.Json" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we want our ref packages to have the same references as the implementation packages?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll clean these up as part of DRI this sprint. Had to include them here to get some build errors addressed. |
||
| </ItemGroup> | ||
|
|
||
| <!-- netstandard dependencies --> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1102,9 +1102,6 @@ | |
| <PackageReference Include="Microsoft.SqlServer.Server" /> | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" /> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
|
|
||
| <!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. --> | ||
| <PackageReference Include="System.Text.Json" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These packages were likely removed due to pruning errors in Visual Studio. This issue has been addressed in #3686, so merging this branch with main should resolve it. |
||
| </ItemGroup> | ||
|
|
||
| <Import Project="$(ToolsDir)targets\GenerateThisAssemblyCs.targets" /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ internal class TransactedConnectionPool | |
| /// A specialized list that holds database connections associated with a specific transaction. | ||
| /// Maintains a reference to the transaction for proper cleanup when the transaction completes. | ||
| /// </summary> | ||
| private sealed class TransactedConnectionList : List<DbConnectionInternal> | ||
| internal sealed class TransactedConnectionList : List<DbConnectionInternal> | ||
| { | ||
| private readonly Transaction _transaction; | ||
|
|
||
|
|
@@ -59,9 +59,6 @@ internal void Dispose() | |
| } | ||
|
|
||
| #region Fields | ||
|
|
||
| private readonly Dictionary<Transaction, TransactedConnectionList> _transactedCxns; | ||
|
|
||
| private static int _objectTypeCount; | ||
| internal readonly int _objectID = System.Threading.Interlocked.Increment(ref _objectTypeCount); | ||
|
|
||
|
|
@@ -79,7 +76,7 @@ internal void Dispose() | |
| internal TransactedConnectionPool(IDbConnectionPool pool) | ||
| { | ||
| Pool = pool; | ||
| _transactedCxns = new Dictionary<Transaction, TransactedConnectionList>(); | ||
| TransactedConnections = new Dictionary<Transaction, TransactedConnectionList>(); | ||
| SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactedConnectionPool|RES|CPOOL> {0}, Constructed for connection pool {1}", Id, Pool.Id); | ||
| } | ||
|
|
||
|
|
@@ -97,6 +94,8 @@ internal TransactedConnectionPool(IDbConnectionPool pool) | |
| /// <value>The IDbConnectionPool instance that owns this transacted pool.</value> | ||
| internal IDbConnectionPool Pool { get; } | ||
|
|
||
| internal Dictionary<Transaction, TransactedConnectionList> TransactedConnections { get; private set; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need private set here. |
||
|
|
||
| #endregion | ||
|
|
||
| #region Methods | ||
|
|
@@ -122,9 +121,9 @@ internal TransactedConnectionPool(IDbConnectionPool pool) | |
| TransactedConnectionList? connections; | ||
| bool txnFound = false; | ||
|
|
||
| lock (_transactedCxns) | ||
| lock (TransactedConnections) | ||
| { | ||
| txnFound = _transactedCxns.TryGetValue(transaction, out connections); | ||
| txnFound = TransactedConnections.TryGetValue(transaction, out connections); | ||
| } | ||
|
|
||
| // NOTE: GetTransactedObject is only used when AutoEnlist = True and the ambient transaction | ||
|
|
@@ -181,10 +180,10 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal | |
| // NOTE: because TransactionEnded is an asynchronous notification, there's no guarantee | ||
| // around the order in which PutTransactionObject and TransactionEnded are called. | ||
|
|
||
| lock (_transactedCxns) | ||
| lock (TransactedConnections) | ||
| { | ||
| // Check if a transacted pool has been created for this transaction | ||
| if ((txnFound = _transactedCxns.TryGetValue(transaction, out connections)) | ||
| if ((txnFound = TransactedConnections.TryGetValue(transaction, out connections)) | ||
| && connections is not null) | ||
| { | ||
| // synchronize multi-threaded access with GetTransactedObject | ||
|
|
@@ -212,14 +211,14 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal | |
| transactionClone = transaction.Clone(); | ||
| newConnections = new TransactedConnectionList(2, transactionClone); // start with only two connections in the list; most times we won't need that many. | ||
|
|
||
| lock (_transactedCxns) | ||
| lock (TransactedConnections) | ||
| { | ||
| // NOTE: in the interim between the locks on the transacted pool (this) during | ||
| // execution of this method, another thread (threadB) may have attempted to | ||
| // add a different connection to the transacted pool under the same | ||
| // transaction. As a result, threadB may have completed creating the | ||
| // transacted pool while threadA was processing the above instructions. | ||
| if (_transactedCxns.TryGetValue(transaction, out connections) | ||
| if (TransactedConnections.TryGetValue(transaction, out connections) | ||
| && connections is not null) | ||
| { | ||
| // synchronize multi-threaded access with GetTransactedObject | ||
|
|
@@ -237,7 +236,7 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal | |
| // add the connection/transacted object to the list | ||
| newConnections.Add(transactedObject); | ||
|
|
||
| _transactedCxns.Add(transactionClone, newConnections); | ||
| TransactedConnections.Add(transactionClone, newConnections); | ||
| transactionClone = null; // we've used it -- don't throw it or the TransactedConnectionList that references it away. | ||
| } | ||
| } | ||
|
|
@@ -296,9 +295,9 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra | |
| // TODO: that the pending creation of a transacted pool for this transaction is aborted when | ||
| // TODO: PutTransactedObject finally gets some CPU time? | ||
|
|
||
| lock (_transactedCxns) | ||
| lock (TransactedConnections) | ||
| { | ||
| if (_transactedCxns.TryGetValue(transaction, out connections) | ||
| if (TransactedConnections.TryGetValue(transaction, out connections) | ||
| && connections is not null) | ||
| { | ||
| bool shouldDisposeConnections = false; | ||
|
|
@@ -318,7 +317,7 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra | |
| if (0 >= connections.Count) | ||
| { | ||
| SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.TransactedConnectionPool.TransactionEnded|RES|CPOOL> {0}, Transaction {1}, Removing List from transacted pool.", Id, transaction.GetHashCode()); | ||
| _transactedCxns.Remove(transaction); | ||
| TransactedConnections.Remove(transaction); | ||
|
|
||
| // we really need to dispose our connection list; it may have | ||
| // native resources via the tx and GC may not happen soon enough. | ||
|
|
@@ -350,4 +349,4 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra | |
| } | ||
|
|
||
| #endregion | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this related to adding stress tests for the connection pool?