dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
842 stars 281 forks source link

Feasibility of porting SqlClient to a single-threaded platform (e.g. `wasi-wasm`) #2810

Closed dicej closed 1 month ago

dicej commented 1 month ago

I've recently been working to update .NET's WASI support to target the latest edition of that platform, called WASIp2. The previous edition, WASIp1, had very limited TCP/UDP socket support, which meant it wasn't practical to port libraries like SqlClient to them. However, WASIp2 has much better socket support, and @pavelsavara and I are working on porting System.Net.Sockets to make use of it.

One of our goals is to get popular libraries like this one working. To that end, I've managed to build SqlClient locally for WASI by hacking Microsoft.Data.SqlClient.csproj a bit to make it think WASI is just another Unix platform (which it is for many purposes). However, opening a connection to a database via SqlConnection.OpenAsync is currently failing due to an attempt to start a thread in DbConnectionPool.TryGetConnection. WASI does not yet support multithreading, so Thread.Start doesn't work, meaning single-threaded async/await is the only practical way to achieve concurrency (see https://github.com/dotnet/runtimelab/pull/2614 for details).

Here are my questions:

Thanks!

dicej commented 1 month ago

Answering the last question myself: adding Pooling=false; to the connection string passed to the SqlConnection constructor seems to do the job. Which means the other questions are less urgent, but I'm still curious.

David-Engel commented 1 month ago

@dicej There's a project going on to rewrite the connection pool logic. I would wait for that to complete before making your own changes. Project

From the discussion (#2612), it does appear that removing that thread is a design goal:

  • Rely on tasks and the managed thread pool to schedule background work
    • Reduces complexity and removes hidden thread creation.

CC: @mdaigle

dicej commented 1 month ago

Thanks, @David-Engel ! The second item on that list ("Async open requests are handled serially") is another thing I was going to ask about. Glad to see it's on the agenda!

mdaigle commented 1 month ago

Hey @dicej, to my knowledge the connection pool should be the only spot that requires multithreading currently (ctrl+f for new Thread()). And I can confirm @David-Engel's comment above that removing that is a design goal.

dicej commented 1 month ago

Thanks, @David-Engel and @mdaigle. I'm going to close this since all my questions are answered. I'll open a separate issue about adding WASI support for this library (at least with "Pooling=false") once Pavel and I have System.Net.Sockets working. That should amount to a build file tweak at most, I expect.