search

ServiceStack / Issues

Issue Tracker for the commercial versions of ServiceStack
11 stars 6 forks source link

PooledRedisClientManager Async Issue when pool exhausted. #755

Closed nesspete closed 3 years ago

nesspete commented 3 years ago

Hi. We switched from StackOverflow Redis to ServiceStack Redis driver 6 or so months ago with good results. Our app does about 2 million Redis writes per minute, and it's not hard to get in a nasty state if anything goes wrong. Moving to ServiceStack async has generally been good, but we've had some issues during high loads and pool exhaustion. In same cases, it's impossible to avoid pool exhaustion, and run unbounded connection counts can run in to other problems (like SNAT exhaustion on Azure).

After digging in to the PooledRedisClientManager code, we noticed that even the Async calls use Monitor.Wait, which ties up the thread until the wait is over. Also, there's some possible locking challenges because the wait is inside the lock.

We made some changes to the code to create a separate Async GetClient method that is fully async at pool limits and uses Async constructs instead of Monitor.Wait. You can see our changes here:

https://github.com/ServiceStack/ServiceStack.Redis/pull/256

I'd love to have this fix as part of the main release, and I think anyone using this driver in Async mode with a limited pool size (read: On azure) with volume is going to benefit from it. But it probably needs some additional work to get it working on ReadOnly clients, too.

Thanks, Pete

mythz commented 3 years ago

Hi Pete,

The PR is much appreciated, thx! It's now merged and available from the latest v5.10.5 that's now available on MyGet.