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

SQL Server Distributed Cache Extensions suffering significant performance issues #28366

Open
markholst opened this issue Nov 19, 2020 · 4 comments
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware feature-caching Includes: StackExchangeRedis and SqlServer distributed caches help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team
Milestone

Comments

@markholst
Copy link

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
dotnet/SqlClient#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
Copy link
Author

markholst commented Nov 19, 2020

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
Copy link
Member

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
Copy link
Author

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!

@JunTaoLuo JunTaoLuo transferred this issue from dotnet/extensions Dec 3, 2020
@JunTaoLuo JunTaoLuo added the feature-caching Includes: StackExchangeRedis and SqlServer distributed caches label Dec 3, 2020
@JunTaoLuo JunTaoLuo added this to the Backlog milestone Dec 3, 2020
@ghost
Copy link

ghost commented Dec 3, 2020

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.

@JunTaoLuo JunTaoLuo added the help wanted Up for grabs. We would accept a PR to help resolve this issue label Dec 3, 2020
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 2, 2023
@mkArtakMSFT mkArtakMSFT added help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team and removed help wanted Up for grabs. We would accept a PR to help resolve this issue labels Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware feature-caching Includes: StackExchangeRedis and SqlServer distributed caches help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team
Projects
None yet
Development

No branches or pull requests

5 participants