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 https://github.com/dotnet/SqlClient/pull/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 https://github.com/dotnet/efcore/issues/7283 goes into this in detail.

[ajcvickers]

/cc @SamMonoRT

[roji]

@cheenamalhotra in addition to #7283, see https://github.com/dotnet/SqlClient/issues/29 and https://github.com/dotnet/SqlClient/pull/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: https://github.com/dotnet/SqlClient/issues/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 https://github.com/dotnet/SqlClient/pull/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

[ajcvickers]

Verified this is fixed with SqlClient 5.2.1.

[EvgenyMuryshkin]

@ajcvickers which version of EF will get this fix? 8.0.7?

[ErikEJ]

@EvgenyMuryshkin You can just add an explicit PackageReference to 5.2.1

[EvgenyMuryshkin]

@ErikEJ I tried but I got lots of failed tests due to transient connection failures, will try to create repro soon

[ajcvickers]

Re-opening to consider updating the 8.0 dependency, which is currently at 5.1.5.

[roji]

@ajcvickers isn't SqlClient planning to revert their change in the 5.1 branch? I haven't been following closely...

[ajcvickers]

@roji Not sure. Will try to pull together the relevant info for the team to discuss.

[ErikEJ]

@ajcvickers @roji This will be fixed in 5.1.6 https://github.com/dotnet/SqlClient/projects/20

[EvgenyMuryshkin]

@ErikEJ looks like my transient connectivity errors were something random and gone after PC restart (Windows update forced me to do that). EF 8.0.6 + direct reference 5.2.1 seems to be working fine. Thanks.

[MichaCo]

Our unit test performance for some apps degraded like crazy after updating from .NET 6 to .NET 8.

After trying many things for hours I finally found this issue and the connection string workaround... At least this time it wasn't the huge performance issue with OPENJSON in EF 8.

Build and test duration jumped from 5 to 20minutes on average for some CI builds (which runs integration tests and create a lot of databases)

This problem still exists today when using EF Core 8.0.6.

Changing the connection string to include ;ConnectRetryCount=0 fixed all the problems... For us this problem only affects unit tests where its fine to disable retry (at least as long as we do not actually use certain features I guess). But it would be really great if that could be fixed in EF 8.0.x soon

[ajcvickers]

Note: this is now fixed for 9.0 RC2 with an LTS SqlClient by changing to SqlClient 5.1.6. We plan to also patch 8.0 for this, but we're going to let 5.1.6 have a bit of time in the wild first.

[AndriySvyryd]

Fixed in https://github.com/dotnet/efcore/commit/576f0d009f9d63fc36fa3d4a4a6c40b8e9d9d93b