- Notifications
You must be signed in to change notification settings - Fork311
Preserve distributed transactions on pooled connection reset#3019
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
/azp run |
| Azure Pipelines successfully started running 1 pipeline(s). |
codecovbot commentedNov 20, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #3019 +/- ##==========================================+ Coverage 72.68% 72.70% +0.01%========================================== Files 283 283 Lines 58975 58976 +1 ==========================================+ Hits 42864 42876 +12+ Misses 16111 16100 -11
Flags with carried forward coverage won't be shown.Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...t.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.Windows.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| @@ -1075,9 +1075,11 @@ private void ResetConnection() | |||
| // distributed transaction - otherwise don't reset! | |||
| if (Is2000) | |||
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.
Leaving Is2000 logic in place for now until we remove support for it. The fix will not apply to versions before 2000.
| [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), Timeout = 10000)] | ||
| public void Test_EnlistedTransactionPreservedWhilePooled() |
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.
Verified that these tests currently fail when run on main against an azure db.
0322d44 intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
The current behavior only preserves transaction state for the root transaction. Enlisted connections that are reset will be placed in the transacted connection pool, but in some situations, will no longer participate in the transaction. The next connection to request the connection will expect it to be participating in the transaction, but actions on the connection will auto-commit.
This changes the reset logic to preserve transaction state for enlisted connections (part of a distributed transaction, but not root).