Announcement: Call for help: System.Data.SqlClient in .NET Core 2.0

[karelz]

We have found quite non-trivial gaps in System.Data.SqlClient based on Preview 1 feedback.

First, thanks everyone helping us discovering them, understand them and fixing them. Also thanks for the patience with the lower-than-desired quality of the area.
Special shout outs to @NickCraver @FransBouma @RickStrahl @cskwg and @cincuranet

We have had several meetings in last few days and we are working closely with the Data.SqlClient team. They kindly heard us out and increased the number of people focusing on .NET Core 2.0 shipping from 1 to 3! So huge thanks to Ken (new manager of the area) for making it happen! And thanks Data team for helping us to get the area in high-quality shape: @saurabh500 @corivera @geleems

Here's update on the remaining work in the space (from @saurabh500):
Query of all SqlClient bugs

We have a PRs planned to address issues in the query above for which we have a definite timelines.

1. PR User message for non-support of Sys.Transactions in SqlClient #21765 - due EOD today
2. PR SqlConnection.GetSchema is missing override in SqlClient #21719 - PR in master, testing EF and Dapper
3. PR netstandard2.0: DataTable isn't usable as a parameter (SqlClient/DataTable issue) #21671 - PR merged in master. Port to 2.0 pending.
4. PR Scan SqlClient for missing overrides and interface implementations #21697 - This is an aggregate of issues that came out due to customer feedback. We are addressing or already have addressed everything mentioned here through other issues.
5. PR SqlConnection doesn't override DbProviderFactory property #21731 - PR up from @FransBouma. Needs to be driven to completion.
6. PR DbConnection doesn't offer an internal property ProviderFactory #21732 - PR up from @FransBouma. Needs to be driven to completion.

The three issues which can consume time beyond next week are

1. Review SqlClient tests on Desktop for scenarios missing from .Net Core #21797 - Review SqlClient tests on Desktop for scenarios missing from .Net Core.
2. Analyze SqlParameter on Desktop to find out scenarios of missing support for DataTable #21776 - Analyze SqlParameter on Desktop to find out scenarios of missing support for DataTable
3. Investigate use of LongRunning continuation option in SqlClient async reads #21739 - Investigate use of LongRunning continuation option in SqlClient async reads

Apart from these above issues, we will be looking at additional testing of .Net Core. In case they uncover any additional issues, we will open them and bring them to Shiproom.

[karelz]

We also acknowledge that our test coverage of System.Data.SqlClient in .NET Core is less than ideal :( (something to address in next version).
Call for help: As such we would welcome any help in the space, incl. feedback from porting more upstack libraries/apps to .NET Core to make sure quality is high. Thank you!

cc @mgravell @roji

[clrjunkie]

@karelz
As I wrote on January 2016, #16067 please reconsider my plea for making stress testing System.Data via Azure test benchmark a requirement. High load testing like that is not something that can be easily setup by the community. I strongly believe this is of utmost importance for detecting stability issues that may surface due to the new managed cross-platform socket implementation in .NET Core.

Furthermore, as I mentioned on may, 2016 in #16650

“*Sql Reference Client - Since virtually all ADO.NET DB Drivers share 80% of the high-level requirements (API Surface, Connection Pooling, retry logic, etc.) It would be wise to invest not only in coding a robust SQL Client for SQLServer but also document and record a code review session so other ADO.NET Driver developers can build upon the existing implementation and focus mainly on the protocol parsing logic.”

I strongly recommended you seize this opportunity to invest in a clean separation of Api surface, connection management logic and most important strict encapsulation of protocol definition and parsing code. A design document for others to reference is also a must.

IMHO, ADO.NET is the BEST (read: THE BEST) relation data access API out there, mainly due to very smart API design which allows developers a high degree of flexibility for building their own Data Access Layer according to their own requirements and personal taste. It will always be known and remembered as a “Microsoft” designed API which is part of .NET and that’s a good thing! It would be wise to treat this as something of strategic importance that goes beyond “a client for SQL Server”. I have no doubt that making the ADO.NET SqlClient a “reference client” for building other “SQL Clients” will rapidly bring new information stores to the .NET eco-system and accelerate the adoption of the .net core platform in general! (This something every developer working with SQL can appreciate!)

[karelz]

@clrjunkie

making stress testing System.Data via Azure test benchmark a requirement

It is a good idea, but not realistic in 2.0 timeframe. It requires non-trivial investment into infrastructure beside just the test assets. Either way, I let Data team to assess feasibility of this approach in Future. It is definitely outside of scope of 2.0, so let's keep it off this issue.

I strongly recommended you seize this opportunity to invest in a clean separation of Api surface,

This is potentially good long-term idea, but it is unrelated to this task as it requires new APIs. In this issue we need Desktop APIs to behave the same on .NET Core 2.0. Let's keep this discussion off this issue please.

[clrjunkie]

@karelz

You wrote:

Apart from these above issues, we will be looking at additional testing of .Net Core.

That's why I thought it's worth to reconsider the Azure benchmark test, maybe the infra is already in place at Azure.

This is potentially good long-term idea, but it is unrelated to this task as it requires new APIs. In this issue we need Desktop APIs to behave the same on .NET Core 2.0. Let's keep this discussion off this issue please.

Just to clarify, my suggestion is not about adding or changing any "Desktop API" but rather about the structure of the SqlClient implementation.. I let the Data team reading this have their take away...

[karelz]

That's why I thought it's worth to reconsider the Azure benchmark test, maybe the infra is already in place at Azure.

I see, now it makes sense. The testing Data teams plans to do is to reuse existing Desktop tests as much as possible. Given we have just 1 week until Escrow, any larger non-tactical investments are great, but not immediately helping. My assumption is that Azure testing infra is multi-man-month if not a man-year investment. Obviously it doesn't fit. If you think there is something simple/easy tactical that can be done on shorter time frame, we are all ears.

Just to clarify, my suggestion is not about adding or changing any "Desktop API" but rather about the structure of the SqlClient implementation

Got it now, thanks. I let Data team to comment on that suggestion. However, any refactoring / large changes to the code are off the table for 2.0, unless they are super-must-haves with clear super-high value and low-risk.
Shipping 2.0 means we need to stop taking risky changes, otherwise we will never stabilize.

Thanks for your suggestions!

[clrjunkie]

Given we have just 1 week until Escrow, any larger non-tactical investments are great, but not immediately helping.

I disagree… This is as tactical you can get. P0 bugs, if any will surface after 5min run… but I now understand the time constraint...

Anyway, I’m adding to this thread all the people listed in the document:

“Azure SQL Database benchmark overview”
https://docs.microsoft.com/en-us/azure/sql-database/sql-database-benchmark-overview

Maybe one of them can help setup a quick sanity run if the test environment is still there…
@rothja @JennieHubbard @CarlRabeler @jan-eng

Shipping 2.0 means we need to stop taking risky changes, otherwise we will never stabilize.

Agree.

p.s

Just to further clarify, my suggestion regarding the "SQL Reference Client" is not about a new ADO.NET Plug-in model or any new public interfaces.

The message is simple: “Here is how we did it, take it from there”

[karelz]

@clrjunkie if there is something simple we can run, I want to know more. My understanding was that we would need to build brand new infra. Can you (or someone) please show us an example what and how to run?
If it is cheap to do (in time invested), we should do it. However, we may not have people available right now to explore, if it is simple to do or not :( ... so any help in this angle would be highly appreciated.
cc @saurabh500 if he heard about this before (I am not Data expert or user myself).

[roji]

One more thought about this - ADO.NET should ideally have a provider test suite (or specification suite), which provides can extend and run - this can help promote uniformity across providers, or at least surface API areas where behavior differs (and help document them). I've suggested this in #17004 but unfortunately have little time to contribute on this right now. This is obviously not a short-term task.

[karelz]

@roji sounds like a good long-term idea. Let's keep that discussion in #17004 - I will ask Data team to review and reconsider many of the old issues once we are done with 2.0 madness :).
Thanks!

[rothja]

@karelz @roji In regards to the benchmark topic. The actual tests were run by the product team, but we helped to publish it. I have an old email thread on this that I can dig up and send you that might provide some context. (Update: I just looked through my old mails on this, and I don't have the mail that identifies where the tests existed or who owned them for the benchmark overview...sorry).

[clrjunkie]

@guyhay In regards to "Azure SQL Database benchmark", Do you know if this test still exist and who may be the right contact for implementation details?

[karelz]

FYI: Feedback from @cincuranet in dotnet/corefx#20309 - thanks!

[NickCraver]

I'd love to test this without crazy feeds in play with our main libraries - is there any chance of a newer preview of System.Data.SqlClient can make its way to NuGet?

Related: I've never been more supporting of an alpha/beta channel on NuGet itself with support in the .csproj than while testing netstandard2.0 :)

[karelz]

I think Preview2 should be out soon. That's the best non-myget option now.

BTW: 2 weeks ago I kicked off internal technical discussion about having regular-ish monthly preview releases on NuGet or on myget in special feed - I just created dotnet/corefx#20911 to track that discussion.

[karelz]

Thanks everyone for your help. I don't think we need this announcement issue around anymore.