spring-session icon indicating copy to clipboard operation
spring-session copied to clipboard

Failover support with sticky session

Open bpapez opened this issue 8 years ago • 15 comments

We have a strong requirement to use sticky sessions, because when content is written to our backend repository there is some asynchronous technology to update other cluster nodes. So only with sticky session it is guaranteed that the writing user can immediately on the next request read the updated content.

Therefore we are currently using the DefaultCookieSerializer and specify a jvmRoute similar to how it is done at Tomcat. However we would like to also provide failover support, so that when a server goes down, another cluster node is taking over the session. As the session is stored in Redis this is perfectly possible. However the problem we are facing is that now the session with the jvmRoute in the cookie still points to the node, which has been shut down.

Lets say I had servers with jvmRoute node1, node2, node3 and then node3 goes down. What happens next is that suddenly the requests will arbitrarily be sent to node1 and node2 (like non-sticky), whereas we would like that the requests of the session continue sticking always to the same server. For that it would be necessary to create a new cookie with the jvmRoute of the new cluster node, which will now continue working on that session.

When using Tomcat PersistentManager this problem is solved by using the org.apache.catalina.ha.session.JvmRouteBinderValve. How could something like that be best solved when using Spring Session?

bpapez avatar Mar 23 '16 15:03 bpapez

I had a related issue just today. When a sessionid includes .node3 jvm route, and node3 goes down. The DefaultCookieSerializer looks up the session id including .node3 because node3 doesn't match the local node's jvmroute. I just made a local change to remove the non-local jvmroute manually, but then hit the issue that you are asking about.

My sessionid still has .node3 in it, and so it will be randomly tossed between node1 and node2 until node3 comes online.

It's interesting, because the Tomcat JvmRouteBinderValve, replaces the route in the sessionid, and continues to use the sessionid with the route in it. That makes it easier to detect this change. SpringSession strips the jvmroute in the CookieSerializer. So the SessionRepositoryFilter.commit() can't tell that the route has changed because it is comparing the sessionid to call httpSessionStrategy.onNewSession()

Maybe if we move the removing of the jvmRoute until we are looking up the session...
+++ b/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java S session = SessionRepositoryFilter.this.sessionRepository - .getSession(sessionId); + .getSession(removeJvmRoute(sessionId));

I will say, I haven't used the multiple session support, so I don't know what the implications are there.

sc-moonlight avatar Mar 23 '16 18:03 sc-moonlight

Thanks for the report. I haven't had time to look into this in detail, but based on your description this does seem like a problem.

Would either of you be interested in submitting a Pull Request?

rwinch avatar Mar 23 '16 22:03 rwinch

I can try, if so it'll be sometime in the next week.

sc-moonlight avatar Mar 24 '16 00:03 sc-moonlight

Created a pull request. It's a small change (and test), but it covers both issues. First, a request from another node, by cleaning off the jvmroute before getting it from the repository. Second, The requestedSessionId still has the jvmroute in the id, but this then triggers the commitSession() to call onNewSession() which pushes down an updated cookie to the client. So the client will now be pinned to the new node. I also ported this to a local 1.1.1-RELEASE branch. If there is a chance of a 1.1.2-RELEASE, I'd like to request this be considered.

sc-moonlight avatar Mar 28 '16 19:03 sc-moonlight

@sc-moonlight Thanks for the PR. I'm not sure how this would work since the DefaultCookieSerializer strips the jvm route off before it is handed back to the CookieHttpSessionStrategy

rwinch avatar Mar 28 '16 19:03 rwinch

@rwinch So, I'm testing with 1.1.1-RELEASE, a Load Balancer in front of Tomcat. Given two nodes app1 and app2. If a request begins on app1, the cookie will end in app1. On the next request, the ".app1" will be removed because it matches this check in DefaultCookieSerializer. if (this.jvmRoute != null && sessionId.endsWith(this.jvmRoute)) {

However, if app1 stops, and the request goes to app2, it will not match that line, because that line only matches on the jvmRoute for it's current node (app1). When that happens, the requestedSessionId still includes the jvmroute, when SessionRepositoryFilter asks for it from the Session repository.

So what I did was prune that jvmRoute just before asking for the session from the repository. So it doesn't matter if it has a jvmRoute in it or not at that point.

This has the side benefit that this now the session id doesn't match the requestedSessionId (with the jvmroute) if (!isRequestedSessionIdValid()|| !session.getId().equals(getRequestedSessionId())) { so it triggers the new cookie to be written to the client.

I started down the path of ALWAYS trimming the jvmroute just based on indexOf "." in the cookie, but the issue is the code still needs to know that I'm on a different node and the cookie needs to be updated. The cookieSerializer is also the only one that knows what about the jvmRoute. And the cookieSerializer is hidden behind the httpSessionStrategy.onNewSession() . So making it more explicit would require some bigger changes.
Maybe something like refactoring this code into something like SessionRepositoryFilter.this.httpSessionStrategy.commitSession(...) and adding a CookieStrategy.isCookieOutofdate(); if (!isRequestedSessionIdValid() || !session.getId().equals(getRequestedSessionId())) { SessionRepositoryFilter.this.httpSessionStrategy.onNewSession(session, this, this.response); }

Recommendation? I could go down that path if you want. Luckily, it's pretty to easy to test, by just editting your cookie to have a non-matching route.

sc-moonlight avatar Mar 28 '16 20:03 sc-moonlight

@sc-moonlight Thanks for the explanation. This makes sense now.

I'd prefer if the Filter is not aware of the JVM route. We also need to ensure that we are passive (if at all possible). At the moment, I'm not sure how to fix this (or if this is possible with the current architecture).

rwinch avatar Mar 28 '16 20:03 rwinch

What do you have in mind by saying "passive"?

I'll take another try at this using the other approach I outlined, and see what that ends up like. I closed the pull request for now.

sc-moonlight avatar Mar 28 '16 20:03 sc-moonlight

@sc-moonlight I mean that when users update they should be able to do so without any changes to their existing code. This means interfaces cannot have new methods, remove methods, change signatures, etc.

rwinch avatar Mar 29 '16 01:03 rwinch

@rwinch I've been over the code a lot today, and unfortunately I can't see a way to do this without those new methods in a couple of interfaces, while keeping the JVMRoute knowledge out of the Filter.

I redid my change locally, so instead of providing a new CookieSerializer that removes the jvmRoute, I replace the sessionRepository with a bean that extends RedisOperationsSessionRepository bean, and remove the jvmRoute in the getSession() call. This works just like the pull request. In that the sessionid from the cookie from a different node, will still have the jvmroute in it, it won't match, and it will resend an updated cookie after the call. @bpapez This might work for you as well...

@Configuration
public class CustomRedisConfiguration extends RedisHttpSessionConfiguration {
    @Bean
    public RedisOperationsSessionRepository sessionRepository(RedisOperations<Object, Object> sessionRedisTemplate, ApplicationEventPublisher applicationEventPublisher) {
        RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(sessionRedisTemplate) {
            public RedisOperationsSessionRepository.RedisSession getSession(String sessionId) {
                if (sessionId != null && sessionId.contains(".")) {
                    sessionId = sessionId.substring(0, sessionId.indexOf("."));
                }
                return super.getSession(sessionId);
            }
        };

sc-moonlight avatar Mar 29 '16 18:03 sc-moonlight

@sc-moonlight Thanks for the updates. It seems like we have some workarounds for this at least. We will have to look into figuring this out in an upcoming release.

rwinch avatar Mar 30 '16 14:03 rwinch

@sc-moonlight Thanks for finding that workaround. I was able to add that method to override getSession also in our Configuration bean, however that meant that I had to move my class into package org.springframework.session.data.redis , because otherwise the RedisSession class was not visible.

bpapez avatar Apr 08 '16 15:04 bpapez

Glad it helped you too. Yes, the package is an unfortunate side effect. I don't see any reason that the getSession() needs to return RedisSession though, so I'd like to update it to be just Session, but since that is an interface change, it'll have to wait until all of the other changes.

sc-moonlight avatar Apr 08 '16 17:04 sc-moonlight

@sc-moonlight Thanks for posting your workaround. I've implemented it, however, I'm unable to override the complete method as it contains some of the private variables from the parent class. From initial testing, it proved to be working, but I'm worried about leaving some codes out.

My custom class

@Configuration
public class CustomRedisConfiguration extends RedisHttpSessionConfiguration {

	@Override
	@Bean
	public RedisOperationsSessionRepository sessionRepository(
			@Qualifier("sessionRedisTemplate") RedisOperations<Object, Object> sessionRedisTemplate,
			ApplicationEventPublisher applicationEventPublisher) {
		
		RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(
				sessionRedisTemplate) {
			@Override
			public RedisOperationsSessionRepository.RedisSession getSession(
					String sessionId) {
				if (sessionId != null && sessionId.contains(".")) {
					sessionId = sessionId.substring(0, sessionId.indexOf("."));
				}
				return super.getSession(sessionId);
			}
		};
		sessionRepository.setApplicationEventPublisher(applicationEventPublisher);

		return sessionRepository;
	}

}

Parent method

	@Bean
	public RedisOperationsSessionRepository sessionRepository(
			@Qualifier("sessionRedisTemplate") RedisOperations<Object, Object> sessionRedisTemplate,
			ApplicationEventPublisher applicationEventPublisher) {
		RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(
				sessionRedisTemplate);
		sessionRepository.setApplicationEventPublisher(applicationEventPublisher);
		sessionRepository
				.setDefaultMaxInactiveInterval(this.maxInactiveIntervalInSeconds);
		if (this.defaultRedisSerializer != null) {
			sessionRepository.setDefaultSerializer(this.defaultRedisSerializer);
		}

		String redisNamespace = getRedisNamespace();
		if (StringUtils.hasText(redisNamespace)) {
			sessionRepository.setRedisKeyNamespace(redisNamespace);
		}

		sessionRepository.setRedisFlushMode(this.redisFlushMode);
		return sessionRepository;
	}

As you can see there are few lines of codes I have to leave out with this solution. Appreciate your thoughts on this.

charithdesilva avatar Jan 19 '17 01:01 charithdesilva

Any progress on this? We are facing a similar issue.

derylspielman avatar Aug 21 '18 00:08 derylspielman