Skip to content

Cosmos: flow transactional batch failures through the execution strategy so they can be retried#38597

Open
AndriySvyryd with Copilot wants to merge 14 commits into
mainfrom
copilot/fix-cosmos-transactional-batch-retry
Open

Cosmos: flow transactional batch failures through the execution strategy so they can be retried#38597
AndriySvyryd with Copilot wants to merge 14 commits into
mainfrom
copilot/fix-cosmos-transactional-batch-retry

Conversation

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #38044

Cosmos transactional batch failures returned a result object from ProcessBatchResponse rather than throwing, so the failure surfaced as a DbUpdateException outside any execution-strategy scope and could never be retried by an ExecutionStrategy (ShouldRetryOn was never called).

Changes

  • Throw wrapped exception on batch failureCosmosClientWrapper.ProcessBatchResponse now computes the errored entries (those whose per-item status code matches the batch-level error code), wraps the CosmosException into a DbUpdateConcurrencyException (PreconditionFailed) or DbUpdateException (Conflict / other), and throws it directly. CosmosTransactionalBatchResult and CosmosTransactionalBatchException are removed.
  • Execute batches once in the clientExecuteTransactionalBatchAsync no longer wraps itself in the execution strategy; it runs once and returns Task (interface updated).
  • Drive retries from SaveChangesAsyncCosmosDatabaseWrapper takes an injected IExecutionStrategy and wraps the whole set of batches in a single ExecuteAsync. A BatchExecutionState.CommittedOperations counter lets a retry skip already-committed operations while preserving the original ordering of single-entry saves interleaved with transactional batches. The execution strategy unwraps DbUpdateException via ExecutionStrategy.CallOnWrappedException to reach the inner CosmosException before calling ShouldRetryOn, so transient failures are correctly identified and retried.
  • Suppressed concurrency consistency — when a DbUpdateConcurrencyException from a batch is suppressed via OptimisticConcurrencyExceptionAsync, RowsAffected is not incremented (matching SaveAsync single-entry behavior), while CommittedOperations still advances so a subsequent retry skips the already-processed operation.

Because a transactional batch is atomic, a failed batch commits nothing and is retried in place; deterministic transaction serialization across attempts makes index-based skipping of committed operations valid.

Result

The repro from the issue now behaves as expected — a transient batch failure reaches the strategy and is retried:

protected override bool ShouldRetryOn(Exception exception)
{
    Console.WriteLine($"Exception: {exception.Message}"); // now written
    return true;
}

Two functional tests verify the behavior:

  • SaveChanges_transactional_batch_failure_flows_through_execution_strategy — a conflicting batch reaches ShouldRetryOn and yields RetryLimitExceededException, with nothing committed.
  • SaveChanges_suppressed_concurrency_exception_in_transactional_batch_reports_zero_rows_affected — a stale-ETag batch update suppressed by an interceptor returns 0 rows affected and leaves the store unchanged.

…angesAsync

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix retry mechanism for transactional batch failures Cosmos: flow transactional batch failures through the execution strategy so they can be retried Jul 9, 2026
Copilot AI requested a review from AndriySvyryd July 9, 2026 01:58
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosTransactionalBatchException.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Cosmos transactional batch failures bypassing the execution strategy retry pipeline by throwing on batch failure (instead of returning a “failed result”) and moving the execution-strategy scope to wrap the full SaveChanges batch sequence, allowing transient failures to reach ShouldRetryOn and be retried.

Changes:

  • Replace the transactional batch “result” return pattern with a CosmosTransactionalBatchException thrown from ProcessBatchResponse.
  • Change ICosmosClientWrapper.ExecuteTransactionalBatchAsync to return Task and execute once (no per-batch execution strategy wrapping in the client).
  • Wrap the full set of transactional batches in a single execution strategy invocation inside CosmosDatabaseWrapper, with retry skipping of already-committed operations, and add a functional test verifying the exception reaches the execution strategy.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/EFCore.Cosmos.FunctionalTests/CosmosTransactionalBatchTest.cs Adds a functional test asserting transactional batch failures flow through the execution strategy and nothing is committed.
src/EFCore.Cosmos/Storage/Internal/ICosmosClientWrapper.cs Updates the transactional batch API to return Task rather than a result object.
src/EFCore.Cosmos/Storage/Internal/CosmosTransactionalBatchException.cs Introduces an internal exception type carrying errored entries and the underlying CosmosException.
src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs Moves execution strategy retry scope to encompass the entire batch set and adds committed-operation skipping across retries.
src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs Executes transactional batches once and throws CosmosTransactionalBatchException on failure (instead of returning a failure result).

Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs Outdated
…e transactional batch

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…fected; add test

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs Outdated
…currencyException for suppression

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs Outdated
@AndriySvyryd
AndriySvyryd marked this pull request as ready for review July 11, 2026 03:43
@AndriySvyryd
AndriySvyryd requested a review from a team as a code owner July 11, 2026 03:43
Copilot AI removed the request for review from Copilot July 14, 2026 21:45
Copilot AI requested a review from AndriySvyryd July 14, 2026 21:46
…NativeAOT-compatible ctor lookup

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 22:16
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs
…atically known

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 22:42
Copilot AI requested a review from AndriySvyryd July 15, 2026 22:42
@AndriySvyryd
AndriySvyryd requested a review from cincuranet July 15, 2026 23:20
@AndriySvyryd
AndriySvyryd enabled auto-merge (squash) July 16, 2026 17:48
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs Outdated
Comment thread src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs Outdated
…ionInternalCtor lookup

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 17, 2026 18:50
auto-merge was automatically disabled July 17, 2026 18:50

Head branch was pushed to by a user without write access

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI review requested due to automatic review settings July 17, 2026 22:51
@AndriySvyryd
AndriySvyryd enabled auto-merge (squash) July 17, 2026 22:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

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.

Cosmos: Transactional batches can't be retried

4 participants