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

The "network stability" / 2.0 / "pipelines" rollup issue #871

Closed
mgravell opened this Issue Jun 28, 2018 · 37 comments

Comments

@mgravell
Copy link
Contributor

mgravell commented Jun 28, 2018

I'm creating this as a placeholder to close (combine) a number of tracked issues that can basically be summarized as:

I get connection stability issues.

(this could also mean unusual timeouts)

This is the number one support query we get, and I could fully understand it if you feel frustrated that we haven't magically fixed it. Well: good news: we haven't been ignoring you. We have been actively working on a major set of changes that overhaul the entire network layer. The deliverables of this work are, in order:

  • network connection stability
  • especially on TLS (cloud), non-Windows systems and .NET Core
  • better protection from thread-pool starvation
  • lower allocations generally
  • better handling of "backlog" network buffers
  • cleaner code

We plan is to release this as the next "major", 2.0.

Current status: "alpha-ish", with a public alpha drop in the next week or so. We intend to dogfood in production at Stack Overflow before we release anything "beta", and finally "stable".


Q: What does this mean for my current bug?

The reality is, with a network overhaul imminent, we're not going to do anything significant "else" to the network code in the 1.* branch, without a very good reason. So: we're going to close all the existing 1.* network stability issues as "see this".

Q: So you don't care about my 1.* bug?

On the contrary. We care so much that we've invested significant time and effort doing this overhaul precisely to fix the issue affecting you.

Other significant changes in 2.0

Packaging

We are getting rid of the StackExchange.Redis / StackExchange.Redis.StrongName duality; it is an unnecessary confusion and overhead. Right now, our guess is to:

  • strong name StackExchange.Redis
  • make StackExchange.Redis.StrongName take a package dependency on StackExchange.Redis
  • make StackExchange.Redis.StrongName an empty shell which just type-forwards to StackExchange.Redis

Target platforms

We currently target .NETStandard 1.5, .NETFramework 4.5 and .NETFramework 4.6. With 2.0 we are planning to streamline the library to target .NETStandard 2.0 only. As a consumer, you can use this from any of:

  • .NET Standard 2.0 or above
  • .NET Framework 4.6.1 or above
  • .NET Core 2.0 or above

If you're stuck on .NET Framework below 4.6.1 - I feel your pain, but it is becoming impossible to support that, sorry.

Technical details

Basically, we're rewriting the network core to utilize Pipelines. Pipelines do many of the same things that we already had code for in StackExchange.Redis, but it does it better, cleaner, and in ways that should work cross-platform. We're also making extensive use of the new Span<T> / Memory<T> features (which are closely related to Pipelines) to reduce allocations, and just a general spruce, tidy, nip and tuck. And just the general benefit of finding a block of time to stare at redis, more redis, and nothing but redis.

Q: Can I start playing with it?

If you really must, I won't stop you - it is the pipelines branch. Naming is hard. Please don't use it in production at this point. If you see any bugs, feel free to log them, but please mention that you're using 2.0 in the title.

Q: 2.0 - so... an API breaking change?

The only API breaking change here so far is the change of strong name; that by itself is a breaking change, assuming you're using the non-strong-named StackExchange.Redis package.

More subtle changes:

  • there is a new overload of SetPop for popping multiple items; because of the implicit conversion between the literal zero and any enum, it is a little unclear to the reader whether db.SetPop(key, 0) pops zero items, or pops 1 item using the default flags. We consider this to be an unlikely usage in both real existing code (passing the enum as a literal zero) and new code (intending to always pop zero items); only the literal zero (0) is affected
  • we are strongly considering removing support for PreserveAsyncOrder; we think this is a significant cause of performance problems; the most common scenario for this flag is for pub/sub, so a new API similar to a queue is being considered, based on Channel<T>; see #877

Q: Will there be any more 1.* builds?

Yes, but probably not very many. My hope is to get the "streams" PR merged (amazing work there, thanks @ttingen) as a 1.* nuget package, at a minimum.


Feedback welcome on any of these plans.

@mgravell mgravell added this to the 2.0 milestone Jun 28, 2018

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Sep 25, 2018

yes, it happened consistently. Actually I am not able to get successful connection at all. the same code worked for 1.2.6 though. I just installed 2.0 version, is there some connection multiplexer changes? or missing lib I need to use for 2.0 instead of 1.2.6?

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Sep 25, 2018

I am using a Azure function to access redis. at starting point just use connection multiplexer to establish connection. is there additional step to get connection from connection pool? let me check the code tomorrow and I can paste it here.

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Sep 25, 2018

but 1.2.6 just works fine. so maybe you can check what could be difference? I mainly want to change to 2.0 because I saw 2.0 manage its dedicated thread pool, so we should have less chance for redis timeout when worker thread > min thread. (https://github.com/StackExchange/StackExchange.Redis/blob/master/docs/Timeouts.md). but dose that mean 2.0 create redis connection in other thread pool than main thread?

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@danmml

This comment has been minimized.

Copy link

danmml commented Sep 25, 2018

To add to this as I was also having a similar issue with Azure Redis caches and upgrading to 2.0.505, adding in the following before the ConnectionMultiplexer.Connect call made it more clear what the binding redirect issue was:

AppDomain.CurrentDomain.FirstChanceException += (sender, eventArgs) =>
{
Console.WriteLine(eventArgs.Exception.ToString());
};

@mgravell Is it possible to surface these binding errors up to the initial Connect call? This would make it more clear to others trying to upgrade to > 2.0 what the underlying issue is. Even adding in the Console writer as suggested above still writes out a generic error about being "Unable to connect" to the server.

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 25, 2018

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Sep 25, 2018

Great, Looking forward the fix. BTW, in 2.0, can we get rid of too many worker thread causing timeout problem as indicated in https://stackexchange.github.io/StackExchange.Redis/Timeouts?

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 26, 2018

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Sep 26, 2018

Sounds good, which version will have your fix?

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 26, 2018

https://www.nuget.org/packages/StackExchange.Redis/2.0.510 - sorry, I should have updated the release notes - will do in the morning

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Sep 27, 2018

@danmml we're pushing out another version soon which should fix this assembly binding problem

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 3, 2018

@mgravell : Can we use 2.0.510? Is it a stable version without any known issues.

@NickCraver

This comment has been minimized.

Copy link
Member

NickCraver commented Oct 3, 2018

@visharm We're using 2.0.513 in production at Stack Overflow right now. We will strive to never knowingly push a build with issues to NuGet as stable :)

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Oct 3, 2018

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 3, 2018

Okay, do you plan to fix it in the coming versions soon? if so when?

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Oct 3, 2018

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 3, 2018

yes the assembly binding issue

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Oct 3, 2018

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 3, 2018

Oh so its already fixed in 513? But I thought you mentioned even 513 has some assembly binding issues.
Your reply above:
I'd rather you used 513. It took a couple of builds to really nail the
assembly binding problem

@mgravell

This comment has been minimized.

Copy link
Contributor

mgravell commented Oct 3, 2018

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 3, 2018

Okay thanks

@zhangruiskyline

This comment has been minimized.

Copy link

zhangruiskyline commented Oct 5, 2018

Just want to know whether stackexchange 2.0 has supported the reconnect function mentioned in

https://gist.github.com/JonCole/925630df72be1351b21440625ff2671f#reconnecting-with-lazyt-pattern

@visharm

This comment has been minimized.

Copy link

visharm commented Oct 8, 2018

Hi All,

I noticed the latest version of StackExchange.Redis is 2.0.513.
Do we have a corresponding update to the strong name Nuget also?
StackExchange.Redis.StrongName.

@karaziox

This comment has been minimized.

Copy link

karaziox commented Oct 8, 2018

As per the release notes for 2.0, at https://stackexchange.github.io/StackExchange.Redis/ReleaseNotes#20495, the StackExchange.Redis.StrongName is deprecated.
The "StackExchange.Redis" package is now strongly named, you should be using that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment