Allow extension of Lettuce Connection and Subscription classes [DATAREDIS-976]

[spring-projects-issues]

Mark Paluch opened DATAREDIS-976 and commented

As part of scaling up our cloud infrastructure, we identified the need to migrate to clustered Redis for our services deployment that uses Spring Session. We are using

Spring Cloud Finchley.SR2
Spring Boot 2.0.5.RELEASE
Spring Session 2.1.4.RELEASE
Spring Data Redis 2.1.5.RELEASE
Lettuce 5.1.4.RELEASE

Testing with the Redis cluster showed that our designated service that processes events from Spring Session was not receiving any session expiration events. Looking closer, Redis keyspace event notification subscriptions required by Spring Session were not being registered on the Redis cluster master nodes. The subscriptions were present in the cluster slave nodes and were indicated in the info for the connections to the slaves, but were missing from the masters.

We looked into the Spring code and the Lettuce documentation and it appears that some of the code Lettuce uses to handle PubSub with clusters hasn't been integrated yet. The documentation in question is here https://github.com/lettuce-io/lettuce-core/wiki/Pub-Sub#redis-cluster.

We successfully integrated the Lettuce code into a branch of Spring Data Redis based on the 2.1.5.RELEASE and Spring Session began working as expected -- our service once again was receiving the session expiration events that are based on the Redis keyspace event notifications, and we can see the PubSub subscriptions on the Redis master nodes.

The Lettuce documentation specifies different code paths for user-space versus keyspace notifications from Redis. For our purposes we hard-coded the modifications to work as required by Spring Session (i.e., for keyspace notifications), but this fork has some additional code to expose the necessary flag to differentiate the two cases. A change will be required in Spring Session to utilize this new flag.

Caveats:
I haven't contributed before so I am looking for feedback/guidance.
This code has been tested as part of our branch based on the 2.1.5.RELEASE. For the fork, I have integrated it into 2.2.0-SNAPSHOT but we haven't tried this upgrade yet.
I am not sure how the unit tests for cluster code work, as I only have a standalone Redis installation in my local dev environment, so I have not attempted to add any unit tests for this functionality.

Thank you

---

Issue Links:

• DATAREDIS-1123 Clustered Redis keyspace notifications are only received from one node in a cluster

Referenced from: pull request #457

[spring-projects-issues]

Mark Paluch commented

Cluster Pub/Sub works similar to Standalone Pub/Sub except for Keyspace notifications. Keyspace events are not propagated across the entire cluster but remain node-local. This introduces a few challenges:

1. To which nodes should we listen to (probably all master nodes that have slots assigned). For the simple approach, we can go with just all known master nodes
2. How to adapt to cluster reconfiguration during runtime (i.e. without restarting the application): We don't have a solution for that yet.
3. Referring to 1, where we PSUBSCRIBE to all master nodes to listen to keyspace events, we also get notified for non-keyspace events (i.e. all regular Pub/Sub messages). The actual problem is that due to listening to each individual node, we receive a single Pub/Sub message multiple times (e.g. 7 master nodes so we receive the same event 7 times because we have to subscribe to each individual node). Pub/Sub messages have no identity and with a cluster's distributed nature we have no reliable way to de-deduplicate messages.

The last point is actually something that worries me because this can easily lead to surprises in mixed setups (listening for keyspace notifications and consuming regular Pub/Sub messages). Beyond that, we also need an implementation for Reactive Pub/Sub