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

Updated to .Net and made main command readers use async /await #169

Closed
wants to merge 8 commits into from

Conversation

daveTechLed
Copy link

No description provided.

@daveTechLed
Copy link
Author

Oh, .NET9 is not supported, any problem adding it ?

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 29, 2024

Let's stay on .NET 8 LTS pls

@daveTechLed
Copy link
Author

Done, back on 8

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 29, 2024

Why the async commands?

@daveTechLed
Copy link
Author

There are "scalability" problems as pointed out by Brent O, i would suspect using threads that block rather than async could contribute... I have a few other ideas on how to enhance the product too if you want to mail:[email protected] to discuss

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 29, 2024

Do you have a link to the Brent O statement?

@BrentOzar ?

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 29, 2024

@daveTechLed Have you done any benchmarking?

@BrentOzar
Copy link
Contributor

BrentOzar commented Nov 29, 2024 via email

@daveTechLed
Copy link
Author

I don’t have any complaints, works fine for me. Not sure what that reference is to.---Tiny glass keyboardTypos flowing like riversAs winter snow thawsOn Nov 29, 2024, at 9:37 AM, Erik Ejlskov Jensen @.> wrote: Do you have a link to the Brent O statement? @BrentOzar ? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.>

https://www.brentozar.com/archive/2019/04/free-sql-server-load-testing-tools/

" it’s not unusual to hit ASYNC_NETWORK_IO waits with SQLQueryStress when it’s unable to keep up with digesting the results"

My educated guess is that moving to Async/Await will help.

@daveTechLed
Copy link
Author

@daveTechLed Have you done any benchmarking?

Nope, but i could try to set something up.

@BrentOzar
Copy link
Contributor

BrentOzar commented Nov 29, 2024 via email

@ErikEJ
Copy link
Owner

ErikEJ commented Nov 30, 2024

@daveTechLed

I have a few other ideas on how to enhance the product too

If you have suggestions, feel free to open new issues here and we can discuss in the open.

Are you the maintainer of TSQLSmells code analyzers btw? (I ported them to .NET 6+)

My educated guess is that moving to Async/Await will help.

Actually, SqlClient has severe performance issues reading large columns async, so it could become worse. See dotnet/SqlClient#593

@daveTechLed
Copy link
Author

Closed now, a quick example is in my repo, i've added a Gannt chart to represent executions.. Next will be calling the SQL statements setting CONTEXT_INFO. I'll grab various extended events from the server and then should be able to correlate the two so that when you hover over the execution the relevant XE's are shown

@ErikEJ ErikEJ reopened this Nov 30, 2024
@ErikEJ ErikEJ closed this Nov 30, 2024
@daveTechLed
Copy link
Author

@daveTechLed

I have a few other ideas on how to enhance the product too

If you have suggestions, feel free to open new issues here and we can discuss in the open.

Are you the maintainer of TSQLSmells code analyzers btw? (I ported them to .NET 6+)

My educated guess is that moving to Async/Await will help.

Actually, SqlClient has severe performance issues reading large columns async, so it could become worse. See dotnet/SqlClient#593

Ah, i did not know about the large column issue.. sound like a fix is on the way though!

Yes, I am / was the maintainer of TSQLSmells. Using VS as a release vehicle got the better of me though and got a bit crazy finding the right folder for the plugin for the various combinations of VS/SQL that was needed. Do you know if that has changed ? Every once in a while i consider using SSMS/VS for it but never seems an amazing amount of community interest when i raise it.

FWIW : Ive made some mods in my branch above, an XE trace is created and then result captured. The Gannt chart in the bottom pane shows the execution times of each, and selecting an execution will show the XE events (hardcoded for the moment,could use a predefined) captured for that executions CONTEXT_INFO. All very alpha, but even in this state , useful i think :)

@ErikEJ
Copy link
Owner

ErikEJ commented Dec 3, 2024

No, no changes for VS but changes for .NET - you can use NuGet packages now. See my blog posts https://erikej.github.io/dacfx/codeanalysis/sqlserver/2024/04/02/dacfx-codeanalysis.html

Could you share a screenshot of your chart?

@daveTechLed
Copy link
Author

Sure ... here you go
Screenshot 2024-12-03 at 15 17 22
Screenshot 2024-12-03 at 15 18 03

@ErikEJ
Copy link
Owner

ErikEJ commented Dec 3, 2024

@BrentOzar Thoughts? ^^ Maybe slightly out of scope?

@ErikEJ
Copy link
Owner

ErikEJ commented Dec 3, 2024

@daveTechLed Adam has given his sign of approval - looking forward to a PR!

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.

3 participants