Performance decrease (10 second delay) in database migration 8.0.0 vs 8.0.3

[EvgenyMuryshkin]

Hi,

After upgrading from 8.0.0 to 8.0.3, get a large performance decrease during database creation.

https://github.com/EvgenyMuryshkin/EF8PPerf

EF 9 also has the same issue

Include provider and version information

.NET 8/.NET 9
EF 8.0.0 => 8.0.3, 9.0.0-preview.2.24128.4
Windows 11 Pro
VS 2022 17.9.4/17.10.0 Preview 2.0

Thank you

[roji]

@EvgenyMuryshkin looks like a duplicate of #33176, which has already been fixed for 8.0.4. Can you please try the workaround in this comment and confirm?

[EvgenyMuryshkin]

@roji nope, did not work.
I think my issue is not model creation, but database creation.
Model is already created

[roji]

This doesn't repro for me (MacOS Sonoma):

I'm noticing that EF upgraded patch versions of SqlClient between 8.0.0 (SqlClient 5.1.1) and 8.0.3 (SqlClient 5.1.5). Can you try to take a dependency on SqlClient 5.1.1 when using EF Core 8.0.0 to see if the problem comes form there? I'm thinking possibly of #7283 or similar.

[EvgenyMuryshkin]

@roji EF 8.0.1 fast, 8.0.2 slow.

with 8.0.0
5.1.1 - fast
5.1.2 - slow

[roji]

Thanks for confirming!

The 5.1.2 release notes are here, the culprit may be dotnet/SqlClient#1983. This may very well be a dup of #7283.

/cc @David-Engel @cheenamalhotra

[cheenamalhotra]

Have you tried setting ConnectRetryCount=0 on the connection string to disable transient fault handling?

[ajcvickers]

@cheenamalhotra This should not be needed. Issue #7283 goes into this in detail.

[ajcvickers]

/cc @SamMonoRT

[roji]

@cheenamalhotra in addition to #7283, see dotnet/SqlClient#29 and dotnet/SqlClient#463 on the SqlClient side where this was discussed at length (especially why the connection string-based approach is insufficient here).

[EvgenyMuryshkin]

Have you tried setting ConnectRetryCount=0 on the connection string to disable transient fault handling?

it actually worked, thanks :)
for 8.0.3 and 9.0.0 as well

[ErikEJ]

Interestingly, this test is not failing: https://github.com/dotnet/SqlClient/blob/main/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs#L364

[roji]

it actually worked, thanks :)

The problem with setting ConnectRetryCount=0 in your actual connection string is that is disables connection retrying everywhere, not just in the database existence check.

[cheenamalhotra]

@cheenamalhotra in addition to #7283, see dotnet/SqlClient#29 and dotnet/SqlClient#463 on the SqlClient side where this was discussed at length (especially why the connection string-based approach is insufficient here).

Thanks for the context, I think we should also make sure FailFast is supported for OpenAsync() as well, now that Transient fault handling is enabled.. This probably skipped our minds and we didn't realize before.

The problem with setting ConnectRetryCount=0 in your actual connection string is that is disables connection retrying everywhere, not just in the database existence check.

Yes, that is true.

---

For @roji and @ajcvickers
Can you confirm the reason why dotnet/SqlClient#29 regressed was due to the change we made on OpenAsync() API to enable Transient Fault Handling? And that Open() API works as expected with Fail fast option defined?

[cheenamalhotra]

Interestingly, this test is not failing: https://github.com/dotnet/SqlClient/blob/main/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs#L364

Note that it's only testing Open() and not OpenAsync()..

[ErikEJ]

But does OpenAsync support the new overload?

[ErikEJ]

Ah, I found: dotnet/SqlClient#615

[ajcvickers]

Confirmed the regression is for async only.

[EvgenyMuryshkin]

Hi all,

8.0.4 looks good, will run our full test suite to confirm shortly.

Thanks,

[roji]

@EvgenyMuryshkin I don't think we've done anything on the EF side (unless I've missed something) - we're still waiting for support to be added to SqlClient to programmatically opt-out of the retrying behavior (that's dotnet/SqlClient#2433).

[EvgenyMuryshkin]

@roji this is weird. I restarted visual studio and now it is back to 10 second delay... false alarm, sorry about.

[roji]

Note: the plan on the SqlClient side is to revert the retrying behavior for OpenAsync for 5.0 (comment), and re-introduce it in SqlClient 6.0.

Opened #33741 to track eventually using the opt-out when we switch to SqlClient 6.0. We can keep this issue open just to track SqlClient's reverting of the async retry logic in 5, so that people can find it.

[ErikEJ]

In other words, this will be fixed in 5.2.1