Skip to content

SqlClient: Implement optimized version of the new ADO.NET batching API #19

Closed

Description

@GSPP

Edited by @divega on 2/6/2019

We want to add public and provider independent batching APIs to the base classes in ADO.NET. This has been described in detail at https://github.com/dotnet/corefx/issues/35135.

We plan for the batching API to include a "fallback implementation" that should work for most existing providers that simply support the DbCommand functionality. But the fallback implementation will be sub-optimal (it will just concatenate the contents from CommandText into a single string and execute it.

From now on, we are going to use this customer-reported issue to track implementing a SQL Server-optimized version of the batching APIs.

As this issue originally referred to, SqlClient already contain the general functionality necessary in System.Data.SqlClient.SqlCommandSet, but there is no public API to use it directly and the only way to leverage it is through DataSet and SqlDataAdapter.

Original issue contents

--
Command sets are a SQL Server / TDS facility that lets database clients send multiple batches in one round trip. This can be very beneficial for performance. This facility is currently only being used by the SqlDataAdapter infrastructure. This infrastructure is not being used by modern apps anymore and it exposes only a very limited functionality (not arbitrary commands).

Command sets can help with this:

  1. Send multiple batches in one round trip
  2. Receive results independently
  3. Much enhanced performance for executing many small commands
  4. A clean API

SqlCommandSet should be cleaned up and made public. There is great community interest and value in doing this (link to a well known post by Ayende, includes benchmarks, second post, Ayende's hack to pry the class open).

x

Currently, people often concatenate their SQL commands into one batch. This is problematic:

  1. It can defeat plan caching
  2. It's slower
  3. It's tedious and error prone (especially since you need unique parameter names, and errors non-deterministically stop the batch or they don't, and you can't easily find out which command failed)
  4. It is subject to a total 2100 parameter limit (add by @roji, 2020-06-22)

I believe that command sets have great value. It looks like cleaning them up would not be too hard since they were written in a style that looks like they were meant to be exposed.

I also believe that the .NET community is not sufficiently aware that there is this super fast and clean way of submitting many small statements. Many applications and scenarios could benefit from this. When this is made public there should be a blog post on a Microsoft blog about it. It really is a secret ninja trick at this point. "Do this one special trick and get much better performance... DB consultants hate it."

Entity Framework could send updates likes that. According to the measurements by Ayende (and my own) this would be a very nice performance boost. I believe EF sometimes sends multiple sub-queries for one query in case of includes. This could make use of command sets as well cutting away one round trip for each query.

In the age of cloud network latencies are typically higher. It becomes especially valuable to be able to cut out round-trips.

Activity

  1. divega commented on Jul 16, 2018

    @divega

    @GSPP thanks for the detailed post. FYI, https://github.com/dotnet/corefx/issues/3688 is about adding command batching APIs in the ADO.NET provider model. I see this issue as a good complement for SQL Server support of whatever API we come up with.

    Another relevant article about how command batching works in SqlClient: https://blogs.msdn.microsoft.com/dataaccess/2005/05/19/does-ado-net-update-batching-really-do-something/.

  2. changed the title [-]Expose SQL Server Command Sets (System.Data.SqlClient.SqlCommandSet)[/-] [+]SqlClient: Implement optimized version of the new ADO.NET batching API[/+] on Feb 7, 2019
  3. GSPP commented on Feb 7, 2019

    @GSPP
    Author

    I'm pulling over some thoughts from another issue:

    Regarding error handling: It must be possible for providers to keep executing in case of error and keep incurring errors. SQL Server will happily continue in case of many errors. All those errors must be reported the API caller somehow. It must be possible to tell which command incurred what errors (and multiple per command are possible). SqlException could be extended with new information.

    SQL Server commands can also generate info messages. These must be provided as well. The existing event SqlConnection.InfoMessage should handle that but this should be tested to work.

    SQL Server can respond in various ways to errors. It can continue executing the statement, abort the statement but continue the batch, abort the batch or even kill the connection. These must be accounted for and tested. I'm not saying there's a specific problem here. Just wanted to make aware of these different cases so that deliberate choices can be made about them.

  4. roji commented on Feb 7, 2019

    @roji
    Member

    @GSPP it's great to have the various possibilities documented. Just to say that it's not obvious to me that the (default) behavior should be to continue the batch - I'm personally more comfortable with aborting it. But that's indeed a discussion and it may even make sense to allow the user to make a decision here (by exposing some enum on SqlCommandBatch).

  5. GSPP commented on Feb 7, 2019

    @GSPP
    Author

    For what it's worth I think the SQL Server error model is an atrocity. The fact that SQL Server tends to keep running the batch but aborting the transaction means that following statements will execute outside of any transaction causing random damage. But other things could happen as well because, as stated, sometimes the batch is aborted. Sometimes the transaction stays open, sometimes not. It's very random.

    The model is horribly broken in many ways. It seems best to keep command sets as consistent with the single command model as possible. That's the only sanity we can hope to maintain.

  6. roji commented on Feb 7, 2019

    @roji
    Member

    The fact that SQL Server tends to keep running the batch but aborting the transaction means that following statements will execute outside of any transaction causing random damage.

    That's... nuts...... This will all definitely need to be considered carefully when implementing for SqlClient. By the way, is this what currently happens when batching is invoked via the internal SqlCommandSet, which is used via SqlDataSet?

  7. GSPP commented on Feb 7, 2019

    @GSPP
    Author

    I don't know what happens there. My wishlist for this features includes that these cases are tested and documented. When we release the first version of the API any unintended consequences are locked in. There is potential here to lock in really nasty and destabilizing behavior.

    My hunch is that it is best to expose exactly the behavior that SQL Server naturally gives. Any artifical corrections are not going to help much and will just eliminate possible use cases and optimizations. In my view this is a power user API that is supposed to expose maximum performance and flexibility.

    Regarding random damage, SQL Server introduced the XACT_ABORT setting. It reliably aborts batch and transaction. It is default off but it's a great way to obtain a reliable failure mode. There also is TRY-CATCH in T-SQL. I'm sure the team understands that the existing model is bad but of course it is locked in for all time for compatibility.

    I personally avoid transaction and error handling in T-SQL whenever possible. C# is perfectly equipped to do that. But some people are forced to write stored procedures and they have no choice but to include a lot of boiler-plate code into every procedure to obtain sane error semantics. It really is a lot of code duplicated each time.

  8. roji commented on Feb 7, 2019

    @roji
    Member

    At the point where the new ADO.NET batching API is merged and confirmed, and there's question of implementing it in SqlClient, these questions would have to be revisited in detail...

    My hunch is that it is best to expose exactly the behavior that SQL Server naturally gives. Any artifical corrections are not going to help much and will just eliminate possible use cases and optimizations. In my view this is a power user API that is supposed to expose maximum performance and flexibility.

    I have no idea what is natural here. One thing I do believe, is that it makes no sense to implement batching in a way that is inherently unreliable (from a transaction and/or error handling point of view). We also have two batching options that are already implemented by SqlClient today: the concatenation-based one in DbCommand and the internal SqlCommandSet exposed via SqlDataSet. The investigation on possible behaviors and guarantees for the new API should probably start there.

  9. GSPP commented on Feb 8, 2019

    @GSPP
    Author

    By "naturally" I mean this new API should expose whatever the TDS protocol feature for command sets delivers. Likely, we have no choice over the behavior anyway.

    Concatenation based approaches are impossible to make reliable in the general case (for example, CREATE VIEW must be the only statement in the batch). This choice can be discarded immediately. There is little value in that because client code can do that already.

  10. divega commented on May 15, 2019

    @divega

    As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

  11. transferred this issue fromdotnet/corefxon May 15, 2019
  12. bgribaudo commented on May 20, 2019

    @bgribaudo

    @roji [from https://github.com/dotnet/corefx/issues/35135#issuecomment-493700728]

    It is indeed the client's responsibility to drain and process responses coming from the server. This is similar to the PostgreSQL case: when responses aren't drained quickly enough the TCP buffer gets full and the server simply blocks. However, this doesn't really seem specific to batching in any way (can easily occur with a single command where row processing is very slow) - or am I mistaken?

    I believe you are correct--I'm unaware of any way in which this would be batching specific.

  13. roji commented on Aug 9, 2019

    @roji
    Member

    Answering @Wraith2's comment in https://github.com/dotnet/corefx/issues/35135#issuecomment-519721600.

    Sounds like things are somehow being built not against the newest versions? If you want to point me to a PR/branch I can try to take a look.

    In any case, we know that the new batching stuff won't go into .NET Core 3.0 or .NET Standard 2.1. That means it may be OK to just wait for MS.Data.SqlClient to go open-source and work on setting up a benchmarking suite there - I don't think it makes much sense to do this in corefx where System.Data.SqlClient is getting pretty much frozen. It will also probably be easier to work in that repo rather than within corefx, built-wise.

  14. Wraith2 commented on Aug 9, 2019

    @Wraith2
    Contributor

    From a clean corefx master. have a corefx command prompt open at the repo root. Create a new branch.
    Run build src\System.Data.SqlClient and build src\System.Data.Common they will succeed, if It doesn't there are other problems.

    Open System.Data.Common project and add in this code to the ref assembly:

        public abstract class DbBatch : IDisposable, IAsyncDisposable
        {
            public void Dispose() { throw null; }
            protected virtual void Dispose(bool disposing) { throw null; }
            public virtual Threading.Tasks.ValueTask DisposeAsync() { throw null; }
        }

    Then at the command prompt run build src\System.Data.Common again and it will succeed, you'll get ApiCompat errors but those are just because we didn't add an implementation. The compile has succeeded for everything and they're not the same, all good to continue development at that point.

    Open SqlClient project, add this code into the ref assembly:

    namespace System.Data.Common
    {
        public abstract class DbBatch : IDisposable, IAsyncDisposable
        {
            public void Dispose() { throw null; }
            protected virtual void Dispose(bool disposing) { throw null; }
            public virtual Threading.Tasks.ValueTask DisposeAsync() { throw null; }
        }
    }

    Then at the command prompt run build rc\System.Data.SqlClient again, and see the following failures:

    System.Data.SqlClient.cs(246,50): error CS0246: The type or namespace name 'IAsyncDisposable' could not be found (are you missing a using directive or an assembly reference?) [E:\Programming\csharp7\corefx\src\System.Data.SqlClient\ref\System.Data.SqlClient.csproj]
    System.Data.SqlClient.cs(250,40): error CS0234: The type or namespace name 'ValueTask' does not exist in the namespace 'System.Threading.Tasks' (are you missing an assembly reference?) [E:\Programming\csharp7\corefx\src\System.Data.SqlClient\ref\System.Data.SqlClient.csproj]
    

    ValueTask and IAsyncDisposable are both part of core, they're surfaced through System.Runtime type forwards. Both ref projects contain project references to the same project in the src tree for this, and yet one works and the other does not. If you check the .csproj files for both solutions they're almost the same except sqlclient has netfx and netstandard targets, neither of which is the default in the build command (netcoreapp is).

    /pokes @ViktorHofer because you usually help me work out how to build things and @jnm2 because we were discussing this in gitter.

  15. 41 remaining items

  16. Wraith2 commented on Dec 19, 2021

    @Wraith2
    Contributor

    @DavoudEshtehari @JRahnama This is the issue we discussed last week. The api is in net 6 so what we need is a net6 build target and a policy decision from the MS side on the api will be made available in other builds where the base classes aren't available.

    @roji to get this onto the management radar to implement we need to get more upvotes. I don't do twitter etc but is there any chance you or any of the EF team could raise the visibility and as anyone who wants the feature to give us an upvote?

  17. JRahnama commented on Dec 20, 2021

    @JRahnama
    Contributor

    @Wraith2 we start adding net6 for TFWs (sometime before preview1) and after that we are interested to start reviewing this and take it in also before preview 1.

  18. Havunen commented on Sep 27, 2022

    @Havunen

    Any news from the progress of this task?
    .NET6 has been active LTS for some time already.

    We are facing similiar issue as described here: dotnet/efcore#22852

  19. Wraith2 commented on Sep 27, 2022

    @Wraith2
    Contributor

    No news.

  20. Wraith2 commented on Nov 3, 2022

    @Wraith2
    Contributor

    Initial implementation is up: #1825

    Please review, test your use cases, provide (constructive) feedback. I expect there to be at least some further work required and I feel that we could use some crowd sourced tests to provide better behavioural coverage.

  21. dazinator commented on Dec 7, 2023

    @dazinator

    Any more news?

  22. Wraith2 commented on Dec 7, 2023

    @Wraith2
    Contributor

    The feature was merged. It'll be available in netcore only in all future builds.

  23. ErikEJ commented on Dec 9, 2023

    @ErikEJ
    Contributor

    @JRahnama Could this be closed?

  24. JRahnama commented on Dec 9, 2023

    @JRahnama
    Contributor

    Closing as v5.2.0-preview4 is released with this feature included.

  25. Wraith2 commented on Dec 10, 2023

    @Wraith2
    Contributor

    @ayende sqlclient batching api is now public

  26. ErikEJ commented on Dec 13, 2023

    @ErikEJ
    Contributor

    @Wraith2 Just to understand - each SqlBatchCommand supports up to 2098 parameters, is that correct?

  27. Wraith2 commented on Dec 13, 2023

    @Wraith2
    Contributor

    As far as i know, yes. Please feel free to try it...

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

    Performance 📈Issues that are targeted to performance improvements.

    Type

    No type

    Projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions