Skip to content
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

Distinguish cancellation from failure #26988

Merged
merged 2 commits into from
Dec 14, 2021
Merged

Conversation

roji
Copy link
Member

@roji roji commented Dec 13, 2021

  • Instead of QueryIterationFailed, SaveChangesFailed, and CommandError, we now emit QueryCanceled, SaveChangedAsync and CommandCanceled when we detect cancellation. The default log level for the cancellation events is Debug.
  • Cancellation is detected by looking at the exception rather than checking whether the token has been triggered. This allows:
    • Avoid race conditions where a non-cancellation error is bubbling up just as the user triggers the token.
    • Detect cancellations not triggered via the cancellation token (e.g. triggered in the database directly). This is also why the sync execution path is covered.
  • Unfortunately, SqlClient provides no way to detect cancellation exceptions (see Cancelling an async SqlClient operation throws SqlException, not TaskCanceledException SqlClient#26 (comment)). As a result, cancellation detection logic is in a new ExceptionDetector singleton service, overridden by SQL Server to check the cancellation token (so the two problems above exist for SQL Server).

Closes #19526

/cc @ajcvickers for logging infra rocket science 🚀🚀🚀

PS We can consider moving transient exception detection into the new ExceptionDetector as well. The default implementation could call the new DbException.IsTransient, the SQL Server implementation would just contain the logic currently in SqlServerTransientExceptionDetector etc.

@roji roji requested review from ajcvickers and a team December 13, 2021 13:59
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.

Distinguish cancellation from failure to allow different logging strategies
2 participants