Spring-Session does not allow the use of read replicas

osvso commented 5 months ago

Describe the bug At some point around the release of Spring Boot 3, we tracked changes in spring data in the hope that the base implementation would be redesigned, but unfortunately this did not happen. The existing implementation does not allow deterministic use of read replicas. The problem is caused by the fact that when the session is first created, so a request is sent without a session identifier attribute (in any supported form), the implementation will attempt to save the session object twice.

    @Override
    public void save(RedisSession session) {
        if (!session.isNew) {
            String key = getSessionKey(session.hasChangedSessionId() ? session.originalSessionId : session.getId());
            Boolean sessionExists = this.sessionRedisOperations.hasKey(key);
            if (sessionExists == null || !sessionExists) {
                throw new IllegalStateException("Session was invalidated");
            }
        }
        session.save();
    }

On the first attempt, the session still has the isNew flag set to true, which causes it to be saved immediately. The second save is triggered by the `SessionRepositoryFilter' as soon as the response is committed.

As a result, in the context of a single HTTP request, the service will first store the session in Redis (executed against the master node), then check if a session with a given id exists (executed against the read replica), and if it does, it will store it again. Otherwise it throws an IllegalStateException("Session was invalidated").

I've seen this issue was reported earlier by other developers, for more context please check the following items:

To Reproduce Setup a simple app that uses Spring-Session with backed by Redis and read replicas. I'm using AWS Elasticache, when read-replicas are enabled around 2% for requests fail due to replication not being fast enough.

Expected behavior Spring-Session works efficiently and does not attempt to store the session data and almost immediately read right after.

marcusdacoregio commented 4 months ago

Hi, @osvso. Thanks for the report.

As you already mentioned, the two saves are needed and well explained in the related issues and here.

However, I'm pretty sure that we could do better in such scenarios where replication is needed. Maybe one alternative would be to somehow track if the session has been created in the very same request and do not throw the IllegalStateException. Of course this would have to be an opt-in feature since we don't want to affect the current behavior.

Let me know what are your thoughts.

osvso commented 4 months ago

Hi @marcusdacoregio! Thank for your reply and pointing at the other issue. I believe it illustrates the use-case behind two saves neatly 👍

From replication stand-point, two saves is not a problem, but the intermediate lookup is. I'm not sure up to what degree it is a realistic to expect another request being executed referring the newly created session while the original request is still processing and making actual changes to the session itself. I believe it complicates a lot of things as we cannot guarantee the second request will get latest session view.

If we could use the session reference in the scenario when it is newly created through out the request processing time it should aid the replication issue. We can keep the saving mechanism intact.

osvso commented 3 months ago

Hello again @marcusdacoregio!

I find this problem as a bug and not really an enhancement as in the current form the solution cannot be used with replication enabled. Also, we have just tested the ElastiCache Serverless integration from AWS and are experiencing issues from the get-go due to the internal replication - in this setup, we don't even need to configure the reader endpoint.

marcusdacoregio commented 3 months ago

Hi @osvso, thanks for your reply, and sorry for the delay in getting back to you.

We have a couple of other enhancements prioritized for the next milestone release, but I'm planning to revisit this as soon as possible. However, if you are interested you can also start digging into the implementation to see how we can improve that.

spring-projects / spring-session

Spring-Session does not allow the use of read replicas #3026