Develop

[maheshj567]

[aembke]

Just looked at this, thanks for the submission @maheshj567.

One thing jumped out around cache invalidation that I want to clarify though. How would this change work when multiple horizontally scaling servers all use this middleware and share a Redis server?

Let's say in this scenario there's two servers, A and B, with a round robin load balancer in front of them. User Alice makes a request to log in and that request ends up at server A, which adds her session to Redis and the LRU cache. Some time later she logs out and this request ends up at server B. Server B gets her request, has a cache miss, goes to Redis to get the data, finds it there, adds it to the LRU cache, then deletes her session from B's cache and Redis. However, in this scenario server A still has her session in the LRU cache and any subsequent requests that end up at A will still succeed, even though to the rest of the system her session is no longer valid.

The alternative here is to introduce another mechanism for communication between servers in order to invalidate caches. However, in order to avoid tightly coupling servers to each other and to avoid introducing more moving parts this means we'd have to use the Redis publish-subscribe interface. That would then require a breaking change to the API in order to create a new Redis client for subscribing to the channel, as well as some more complexity around managing the clients properly. Additionally, reliably verifying Redis pubsub message delivery without coupling servers to each other can be pretty complex, and I'd prefer to avoid that problem entirely if possible.

The overarching question I have though is why add the LRU cache? Is it to speed up the application by avoiding the RTT to and from Redis?

[maheshj567]

Hello @aembke.

Sorry, the pull request was actually made by mistake, although I'd love to see the change go into the module. I forked your repo, made my changes and pushed them. Not sure if that created the pull request or if I did something inadvertently.

We've actually been using this in production at our company. Ours is a small single server setup, so we are using a very small redis setup. The issue we were facing is that at higher load, redis was taking a lot more time to process session related requests. So I integrated the LRU cache, and we started seeing a more stable performance profile.

I agree this won't work when there are multiple servers involved. One possible solution would be a sticky load balancer, which instead of doing round robin requests, sends incoming requests to server that started the session in the first place. This will be a decent solution for applications with predictable load, but as an open source package, I understand this is not a good addition.

To make this work for multiple servers, we can use redis pub/sub to notify all instances that a session related event has happened.

This PR is definitely an incomplete solution, so if you think this is a good feature to have, we can discuss further and see if the redis pub/sub solution makes sense.