Skip to content

System.Data.Common and SqlClient: Async variants for remaining blocking APIs #18225

Closed

Description

@divega

The following potentially blocking APIs currently don't have async variants:

  1. DbTransaction.Commit()
  2. DbConnection.Close()
  3. DbConnection.BeginTransaction()
  4. DbTransaction.Rollback()

Ideally default implementations should be added to the base classes of the ADO.NET provider model which fallback to the sync implementations (following the same pattern we have for the other async methods) and proper async implementations should be added to SqlClient.

Related to dotnet/SqlClient#65.

Activity

  1. self-assigned this
    on Oct 13, 2016
  2. karelz commented on Nov 10, 2016

    @karelz
    Member

    This is a lot of work (esp. ensuring we do not block anywhere), but if anyone wants to tackle, go for it.

  3. caleblloyd commented on Jun 29, 2017

    @caleblloyd

    I think that these are important standards to put into DbTransaction and DbConnection

    Without these APIs, database agnostic codebases such as Entity Framework have to either:

    1. force Synchronous I/O in Asynchronous codepaths for Transactions and closing Database Connections
    2. come up with some non-standard API to let a database provider make async versions of these calls

    I work on MySqlConnector and we have already implemented BeginTransactionAsync, CommitAsync, and RollbackAsync. There is no good way for database agnostic codebases to take advantage of these APIs, however.

    It's a change that should be made eventually to promote efficient use of I/O, seems like sooner would be better than later

  4. roji commented on Jun 29, 2017

    @roji
    Member

    Npgsql also already has NpgsqlTransaction.CommitAsync(), NpgsqlTransaction.RollbackAsync(). NpgsqlConnection.BeginTransaction() isn't needed specifically in Npgsql since the operation doesn't do I/O, but NpgsqlConnection.CloseAsync() is indeed missing.

    @karelz I think what's needed here isn't actual async implementations in SqlClient - it seems that what's missing is these methods in DbTransaction and DbConnection. Like elsewhere, these async methods would simply call their sync counterparts by default, thus falling back to async not forcing any provider to implement them. But it would be possible to access real async implementations for providers that do implement them (e.g. from EF Core).

    So it's possible this should be split into two issues:

    1. Adding missing async methods to System.Data.Common (pretty trivial), and
    2. Providing real async implementations of those methods in SqlClient (non-trivial)
  5. karelz commented on Jun 30, 2017

    @karelz
    Member

    I let area experts comment: @divega @saurabh500 @corivera ... I was just typing triage meeting notes in November 😉

  6. divega commented on Jun 30, 2017

    @divega
    ContributorAuthor

    @roji I agree in reality lack of these APIs in System.Data.Common and SqlClient are two separate issues. I think we can address this first and independently in System.Data.Common and (sounds like a good PR to work on 😄) and then we can do it in SqlClient when we are ready.

    There isn't much of a precedent of having async APIs in the base classes that are not implemented by SqlClient but that doesn't mean it can't happen moving forward.

    PS: I am not splitting the issue because I don't think there is much value in doing so right now. We should just start with System.Data.Common.

  7. roji commented on Jun 30, 2017

    @roji
    Member

    @divega I'm not saying I have time to do a PR for System.Data.Common, but if I did, here's a question :)

    For this to have any value, it would need to go into .NET Standard 2.1 (e.g. to allow EF Core to use it). Are there any guidelines for how new .NET Standard APIs are submitted, introduced into .NET Framework etc?

  8. divega commented on Jun 30, 2017

    @divega
    ContributorAuthor

    @roji that is an excellent point. @karelz or @terrajobst may be able to point you to detailed guidelines if they are available.

    I believe before the new APIs can become part of a version of .NET Standard they would need to be incorporated in other popular .NET platforms and not just in .NET Core. That probably means that libraries that target .NET Standard won't be able to leverage these API for a long time, however:

    • It seems to be better to start walking that long path than to give up
    • I think for the particular case of EF Core we may not need to wait: as @caleblloyd showed in async transaction APIs efcore#9017, EF Core already puts "relational" façades around the System.Data.Common objects. If we add virtual async method for these operations there then providers will be able to override them to leverage their custom async methods.
  9. karelz commented on Jun 30, 2017

    @karelz
    Member

    Currently we do not have clear plans / guidelines for .NET Standard 2.1. There are opinions, but nothing set to stone yet. I expect the discussion to become more serious during summer ...

  10. divega commented on Feb 1, 2019

    @divega
    ContributorAuthor
  11. transferred this issue fromdotnet/corefxon Jan 31, 2020
  12. added this to the 3.0 milestone on Jan 31, 2020
  13. ghost locked as resolved and limited conversation to collaborators on Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    api-needs-workAPI needs work before it is approved, it is NOT ready for implementationarea-System.Datahelp wanted[up-for-grabs] Good issue for external contributors

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions