search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.44k stars 10.02k forks source link

SQL Server Distributed Cache Extensions suffering significant performance issues #28366

Open markholst opened 3 years ago

markholst commented 3 years ago

Describe the bug

Serious performance issues when reading binary values, when using SQL Server Distributed Cache extension.

To Reproduce

Using SQL Server Distributed Cache, persist a value with an arbitrary value of 15MB. Access the data using the async read operations. Note this takes approximately 15 seconds in our environment.

Expected behavior

This should take milliseconds.

Additional context

I suspect this is caused by an open SQL Client bug mentioned below. Can you please verify if this is the case? Is there a work-around that can be implemented in DatabaseOperations until this is resolved?

https://github.com/dotnet/extensions/blob/master/src/Caching/SqlServer/src/DatabaseOperations.cs, line 229 value = reader.GetFieldValue<byte[]>(Columns.Indexes.CacheItemValueIndex);

Reading binary data asynchronously is extremely slow https://github.com/dotnet/SqlClient/issues/593

It looks like the SqlClient team are suggesting people use the StreamAsync operation as a work-around?

One reasonably successful work-around we have tried is to increase the TDS Packet Size up to 32,767. What are your thoughts on this?

markholst commented 3 years ago

Actually, I've changed our implementation over to use the synchronous Get method. The more I thought about it, the more I realised this is a serious issue for scalability.

It doesn't need to be a large value being cached either, even responses of up to 1MB were enough to cause problems.

Using the asynchronous Get method allocates so much memory for a simple call, it wouldn't take many concurrent requests to take a system down. The numbers in the related article suggest 13GB of memory was churned to service a 20MB request. Yikes!

Given this is a front-end caching solution for ASP.NET Core based API and web applications, this can put an ASP.NET Core site at significant risk of a denial-of-service style attack.

Using the synchronous get method brings it down to < 1sec for a cache read operation.

BrennanConroy commented 3 years ago

The numbers in the related article suggest 13GB of memory was churned to service a 20MB request. Yikes!

Assuming you're referring to this comment, this is only for Full framework it looks like. In a .NET Core app you'll get the lower 30MB allocations.

It looks like SQLClient should fix the issue and we could consider looking into SteamAsync in the meantime.

markholst commented 3 years ago

Thanks @BrennanConroy, I have re-read and reached the same conclusion.

I think if the SqlClient team are going to take a long time, I'd suggest using the synchronous routines, as they're considerably faster!

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.