Skip to content

Keep transaction cleanup independent of request cancellation#5574

Draft
mikaelweave wants to merge 1 commit into
mainfrom
mikaelweave/fix-sql-transaction-cleanup-token
Draft

Keep transaction cleanup independent of request cancellation#5574
mikaelweave wants to merge 1 commit into
mainfrom
mikaelweave/fix-sql-transaction-cleanup-token

Conversation

@mikaelweave
Copy link
Copy Markdown
Contributor

Summary

Splits the SqlServerFhirDataStore failed-transaction cleanup token change out of #5553 for independent review.

Changes

  • SqlServerFhirDataStore.cs: pass CancellationToken.None to MergeResourcesCommitTransactionAsync in the MergeInternalAsync single-transaction catch block.
  • This prevents an already-cancelled request token from skipping server-side transaction failure marking.
  • BundleHandler cancellation response mapping remains in Return 408 for cancelled bundle transaction cleanup #5553.

Testing

  • dotnet build src\Microsoft.Health.Fhir.SqlServer\Microsoft.Health.Fhir.SqlServer.csproj --verbosity minimal
  • git diff --check

Related

Split from #5553 for ADO Bug #188321.

Move the SqlServerFhirDataStore failed-transaction cleanup token change out of PR #5553 so it can be reviewed independently from the BundleHandler cancellation response mapping.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.34%. Comparing base (b696902) to head (41f12fa).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5574      +/-   ##
==========================================
- Coverage   77.37%   77.34%   -0.03%     
==========================================
  Files         993      993              
  Lines       36418    36418              
  Branches     5518     5518              
==========================================
- Hits        28177    28169       -8     
- Misses       6880     6887       +7     
- Partials     1361     1362       +1     

see 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants