StackExchange.Redis icon indicating copy to clipboard operation
StackExchange.Redis copied to clipboard

Adding support for cluster keyspace notifications to subscriber

Open ercangorgulu opened this issue 5 years ago • 3 comments

Adding support for cluster keyspace notifications to subscriber by allowing subscription to multiple nodes

Refactored code for previous pull request based on new changes https://github.com/StackExchange/StackExchange.Redis/pull/813

Fixes: https://github.com/StackExchange/StackExchange.Redis/issues/789

ercangorgulu avatar Jul 22 '20 09:07 ercangorgulu

@NickCraver - Any chance we can get this merged? Getting keyspace notification support would be amazing.

shaunco avatar Jun 08 '22 21:06 shaunco

@shaunco Unfortunately not - we've been heads down on some other bits here but the reason this wasn't taken originally was pub/sub was screwed in ways we didn't understand and took a decent deep dive to figure out recently. As a result, the entirely of pub/sub code has been rewritten to behave and hash correctly, fundamentally breaking this PR as well (but fixing all existing use cases).

We're not against adding this feature, but it has a critical component of subscribing this specific thing to all servers which isn't how the rest of subscriptions work. We'd need to handle this case including how to subscribe and re-subscribe since it behaves differently at every level (rather than opportunistically subscribing to any viable server when problems occur like other subscriptions).

Now that we have subscriptions as a base stable and working, this is much more viable to approach, but no time allocation from Marc and I at the moment.

@ercangorgulu We really do appreciate the effort here - hopefully the above makes sense on why changing an unstable system is not desirable (and I should have written this a long time ago). Now that we have the base stable, this is a problem we could go after.

NickCraver avatar Jun 14 '22 14:06 NickCraver

@NickCraver thank you for your feedback. I know this feature is so niche, but we needed to use in our app. Since I already did this for us, I wanted to contribute too. But after a year or so we also stopped using that. Previously cache was stored in memory and redis. So we needed to update those automatically. For some parts we wrote additional layer so it handles this with regular pubsub and for other parts we are just storing required caches in request scope. So basically we removed our dependency of this feature.

ercangorgulu avatar Jun 14 '22 14:06 ercangorgulu