Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jvmRoute as part of the Redis key prevents failover to other nodes #67

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

jvmRoute as part of the Redis key prevents failover to other nodes #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a9e75e
RedisSessionManager now uses different values as external and interna…
May 5, 2015
e438a1c
Idea project files ignored
May 5, 2015
46cc72e
Use the internal representation of the key in all queries to redis
May 7, 2015
2cc7d09
Improved logging in case of exceptions
May 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ example-app/.gradle/*
.DS_Store
.rspec
*.iml
*.ipr
*.iws
.idea/*
57 changes: 31 additions & 26 deletions src/main/java/com/orangefunction/tomcat/redissessions/RedisSessionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
import org.apache.catalina.Session;
import org.apache.catalina.session.ManagerBase;

import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.apache.commons.pool2.impl.BaseObjectPoolConfig;

import redis.clients.util.Pool;
import redis.clients.jedis.JedisPool;
import redis.clients.jedis.JedisSentinelPool;
Expand All @@ -22,7 +19,6 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Set;
import java.util.EnumSet;
Expand Down Expand Up @@ -341,17 +337,20 @@ public Session createSession(String requestedSessionId) {
Jedis jedis = null;
try {
jedis = acquireConnection();

String redisSessionIdKey = null;
// Ensure generation of a unique session identifier.
if (null != requestedSessionId) {
sessionId = sessionIdWithJvmRoute(requestedSessionId, jvmRoute);
if (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L) {
sessionId = requestedSessionId;

redisSessionIdKey = stripJvmRoute(requestedSessionId, jvmRoute);
if (jedis.setnx(redisSessionIdKey.getBytes(), NULL_SESSION) == 0L) {
sessionId = null;
}
} else {
do {
sessionId = sessionIdWithJvmRoute(generateSessionId(), jvmRoute);
} while (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L); // 1 = key set; 0 = key already existed
sessionId = generateSessionId();
redisSessionIdKey = stripJvmRoute(sessionId, jvmRoute);
} while (jedis.setnx(redisSessionIdKey.getBytes(), NULL_SESSION) == 0L); // 1 = key set; 0 = key already existed
}

/* Even though the key is set in Redis, we are not going to flag
Expand Down Expand Up @@ -381,7 +380,7 @@ This ensures that the save(session) at the end of the request
try {
error = saveInternal(jedis, session, true);
} catch (IOException ex) {
log.error("Error saving newly created session: " + ex.getMessage());
log.error(String.format("Error saving newly created session [%s]: %s", sessionId, ex.getMessage()), ex);
currentSession.set(null);
currentSessionId.set(null);
session = null;
Expand All @@ -396,10 +395,10 @@ This ensures that the save(session) at the end of the request
return session;
}

private String sessionIdWithJvmRoute(String sessionId, String jvmRoute) {
if (jvmRoute != null) {
String jvmRoutePrefix = '.' + jvmRoute;
return sessionId.endsWith(jvmRoutePrefix) ? sessionId : sessionId + jvmRoutePrefix;
String stripJvmRoute(String sessionId, String jvmRoute) {
if (jvmRoute != null && sessionId != null) {
final int index = sessionId.indexOf('.');
sessionId = index != -1 ? sessionId.substring(0, index) : sessionId;
}
return sessionId;
}
Expand All @@ -414,7 +413,7 @@ public void add(Session session) {
try {
save(session);
} catch (IOException ex) {
log.warn("Unable to add to session manager store: " + ex.getMessage());
log.warn("Unable to add to session manager store: " + ex.getMessage(), ex);
throw new RuntimeException("Unable to add to session manager store.", ex);
}
}
Expand Down Expand Up @@ -499,10 +498,11 @@ public byte[] loadSessionDataFromRedis(String id) throws IOException {
Boolean error = true;

try {
log.trace("Attempting to load session " + id + " from Redis");
final String redisSessionIdKey = stripJvmRoute(id, getJvmRoute());
log.trace("Attempting to load session " + id + " with key " + redisSessionIdKey + " from Redis");

jedis = acquireConnection();
byte[] data = jedis.get(id.getBytes());
byte[] data = jedis.get(redisSessionIdKey.getBytes());
error = false;

if (data == null) {
Expand All @@ -522,7 +522,7 @@ public DeserializedSessionContainer sessionFromSerializedData(String id, byte[]

if (Arrays.equals(NULL_SESSION, data)) {
log.error("Encountered serialized session " + id + " with data equal to NULL_SESSION. This is a bug.");
throw new IOException("Serialized session data was equal to NULL_SESSION");
throw new IOException(String.format("Serialized session data for the session id [%s] was equal to NULL_SESSION", id));
}

RedisSession session = null;
Expand Down Expand Up @@ -583,15 +583,18 @@ protected boolean saveInternal(Jedis jedis, Session session, boolean forceSave)

RedisSession redisSession = (RedisSession)session;

final String sessionId = redisSession.getId();
final String redisStorageKey = stripJvmRoute(sessionId, getJvmRoute());

if (log.isTraceEnabled()) {
log.trace("Session Contents [" + redisSession.getId() + "]:");
log.trace("Session Contents [" + sessionId + "]:");
Enumeration en = redisSession.getAttributeNames();
while(en.hasMoreElements()) {
log.trace(" " + en.nextElement());
}
}

byte[] binaryId = redisSession.getId().getBytes();
final byte[] binaryId = redisStorageKey.getBytes();

Boolean isCurrentSessionPersisted;
SessionSerializationMetadata sessionSerializationMetadata = currentSessionSerializationMetadata.get();
Expand Down Expand Up @@ -623,15 +626,14 @@ protected boolean saveInternal(Jedis jedis, Session session, boolean forceSave)
log.trace("Save was determined to be unnecessary");
}

log.trace("Setting expire timeout on session [" + redisSession.getId() + "] to " + getMaxInactiveInterval());
log.trace("Setting expire timeout on session [" + sessionId + "] to " + getMaxInactiveInterval());
jedis.expire(binaryId, getMaxInactiveInterval());

error = false;

return error;
} catch (IOException e) {
log.error(e.getMessage());

log.error(String.format("Exception encountered when performing saveInternal with the key [%s]", session.getId()), e);
throw e;
} finally {
return error;
Expand Down Expand Up @@ -664,16 +666,19 @@ public void remove(Session session, boolean update) {
public void afterRequest() {
RedisSession redisSession = currentSession.get();
if (redisSession != null) {
final String sessionId = redisSession.getId();
final boolean validSession = redisSession.isValid();

try {
if (redisSession.isValid()) {
if (validSession) {
log.trace("Request with session completed, saving session " + redisSession.getId());
save(redisSession, getAlwaysSaveAfterRequest());
} else {
log.trace("HTTP Session has been invalidated, removing :" + redisSession.getId());
remove(redisSession);
}
} catch (Exception e) {
log.error("Error storing/removing session", e);
log.error(String.format("Error %s session with the id [%s]", validSession ? "storing" : "removing", sessionId), e);
} finally {
currentSession.remove();
currentSessionId.remove();
Expand Down Expand Up @@ -703,7 +708,7 @@ private void initializeDatabaseConnection() throws LifecycleException {
connectionPool = new JedisPool(this.connectionPoolConfig, getHost(), getPort(), getTimeout(), getPassword());
}
} catch (Exception e) {
e.printStackTrace();
log.error("Exception while initializing JedisPool", e);
throw new LifecycleException("Error connecting to Redis", e);
}
}
Expand Down
56 changes: 56 additions & 0 deletions src/test/java/com/orangefunction/tomcat/redissessions/RedisSessionManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.orangefunction.tomcat.redissessions;

import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* User: Kai Inkinen <[email protected]>
* Date: 2015.05.04
* Time: 10:15
*/
public class RedisSessionManagerTest {

private RedisSessionManager manager;
private String[] randomJvmRoutes = new String[]{"abc", "123", "abc123", "......", "1.1.2.jvmRoute", ",.,..,.,"};

@Before
public final void setupRedisSessionManagerTest() {
manager = new RedisSessionManager();
}

@Test
public void sessionIdWithoutJvmRouteIsReturned() {
assertEquals("session", manager.stripJvmRoute("session", "jvmRoute"));
}

@Test
public void nullReturnedAsIs() {
assertNull(manager.stripJvmRoute(null, "jvmRoute"));
assertNull(manager.stripJvmRoute(null, null));
}

@Test
public void sessionIdIsNotModifiedIfWeDontHaveAJvmRoute() {
for (String jvmRoute : randomJvmRoutes) {
assertEquals("session." + jvmRoute, manager.stripJvmRoute("session." + jvmRoute, null));
}
}

@Test
public void sessionIdWithAnyJvmRouteIsModified() {
for (String jvmRoute : randomJvmRoutes) {
assertEquals("session", manager.stripJvmRoute("session." + jvmRoute, "unknown" + jvmRoute));
}
}

@Test
public void generatedSessionStripsJvmRoute() {
for (String jvmRoute : randomJvmRoutes) {
assertEquals("session", manager.stripJvmRoute("session." + jvmRoute, jvmRoute));
}
}

}