Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #4156 Remove use of PlaceHolderSession for simultaneous session loading (#4304) 

* Issue #4156 Remove use of PlaceHolderSession for simultaneous session loading.
This commit is contained in:
Jan Bartel 2019-11-18 09:06:38 +11:00 committed by GitHub
parent e69eea86c8
commit 318246eb24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 84 additions and 227 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
jetty-memcached/jetty-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/session/TestMemcachedSessions.java
View File

@ -20,6 +20,7 @@ package org.eclipse.jetty.memcached.session;
import java.io.IOException;
import java.io.PrintWriter;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@ -31,12 +32,9 @@ import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.server.NetworkConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.session.AbstractSessionCache;
import org.eclipse.jetty.server.session.CachingSessionDataStore;
import org.eclipse.jetty.server.session.NullSessionCache;
import org.eclipse.jetty.server.session.NullSessionDataStore;
import org.eclipse.jetty.server.session.Session;
import org.eclipse.jetty.server.session.SessionData;
import org.eclipse.jetty.server.session.SessionHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.junit.jupiter.api.Test;
@ -88,56 +86,6 @@ public class TestMemcachedSessions
}
}
public static class NullSessionCache extends AbstractSessionCache
{
public NullSessionCache(SessionHandler handler)
{
super(handler);
}
@Override
public void shutdown()
{
}
@Override
public Session newSession(SessionData data)
{
return new Session(_handler, data);
}
@Override
public Session newSession(HttpServletRequest request, SessionData data)
{
return new Session(_handler, request, data);
}
@Override
public Session doGet(String id)
{
return null;
}
@Override
public Session doPutIfAbsent(String id, Session session)
{
return null;
}
@Override
public boolean doReplace(String id, Session oldValue, Session newValue)
{
return true;
}
@Override
public Session doDelete(String id)
{
return null;
}
}
@Test
public void testMemcached() throws Exception
{

148
jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java
View File

@ -21,6 +21,8 @@ package org.eclipse.jetty.server.session;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;
import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.util.StringUtil;
@ -133,6 +135,18 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
* @return null if the session wasn't already in the map, or the existing entry otherwise
*/
protected abstract Session doPutIfAbsent(String id, Session session);
/**
* Compute the mappingFunction to create a Session object iff the session
* with the given id isn't already in the map, otherwise return the existing Session.
* This method is expected to have precisely the same behaviour as
* {@link java.util.concurrent.ConcurrentHashMap#computeIfAbsent}
*
* @param id the session id
* @param mappingFunction the function to load the data for the session
* @return an existing Session from the cache
*/
protected abstract Session doComputeIfAbsent(String id, Function<String, Session> mappingFunction);
/**
* Replace the mapping from id to oldValue with newValue
@ -152,22 +166,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
*/
public abstract Session doDelete(String id);
/**
* PlaceHolder
*/
protected class PlaceHolderSession extends Session
{
/**
* @param handler SessionHandler to which this session belongs
* @param data the session data
*/
public PlaceHolderSession(SessionHandler handler, SessionData data)
{
super(handler, data);
}
}
/**
* @param handler the {@link SessionHandler} to use
*/
@ -343,113 +341,53 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
protected Session getAndEnter(String id, boolean enter) throws Exception
{
Session session = null;
Exception ex = null;
while (true)
session = doComputeIfAbsent(id, k ->
{
session = doGet(id);
if (LOG.isDebugEnabled())
LOG.debug("Session {} not found locally in {}, attempting to load", id, this);
if (_sessionDataStore == null)
break; //can't load any session data so just return null or the session object
if (session == null)
try
{
if (LOG.isDebugEnabled())
LOG.debug("Session {} not found locally in {}, attempting to load", id, this);
//didn't get a session, try and create one and put in a placeholder for it
PlaceHolderSession phs = new PlaceHolderSession(_handler, new SessionData(id, null, null, 0, 0, 0, 0));
Lock phsLock = phs.lock();
Session s = doPutIfAbsent(id, phs);
if (s == null)
Session s = loadSession(k);
if (s != null)
{
//My placeholder won, go ahead and load the full session data
try
try (Lock lock = s.lock())
{
session = loadSession(id);
if (session == null)
{
//session does not exist, remove the placeholder
doDelete(id);
phsLock.close();
break;
}
try (Lock lock = session.lock())
{
//swap it in instead of the placeholder
boolean success = doReplace(id, phs, session);
if (!success)
{
//something has gone wrong, it should have been our placeholder
doDelete(id);
session = null;
LOG.warn("Replacement of placeholder for session {} failed", id);
phsLock.close();
break;
}
else
{
//successfully swapped in the session
session.setResident(true);
if (enter)
session.use();
phsLock.close();
break;
}
}
}
catch (Exception e)
{
ex = e; //remember a problem happened loading the session
doDelete(id); //remove the placeholder
phsLock.close();
session = null;
break;
s.setResident(true); //ensure freshly loaded session is resident
}
}
else
{
//my placeholder didn't win, check the session returned
phsLock.close();
try (Lock lock = s.lock())
{
//is it a placeholder? or is a non-resident session? In both cases, chuck it away and start again
if (!s.isResident() || s instanceof PlaceHolderSession)
{
session = null;
continue;
}
//I will use this session too
session = s;
if (enter)
session.use();
break;
}
if (LOG.isDebugEnabled())
LOG.debug("Session {} not loaded by store", id);
}
return s;
}
else
catch (Exception e)
{
//check the session returned
try (Lock lock = session.lock())
{
//is it a placeholder? or is it passivated? In both cases, chuck it away and start again
if (!session.isResident() || session instanceof PlaceHolderSession)
{
session = null;
continue;
}
LOG.warn("Error loading session {}", id, e);
return null;
}
});
//got the session
if (enter)
session.use();
break;
if (session != null)
{
try (Lock lock = session.lock())
{
if (!session.isResident()) //session isn't marked as resident in cache
{
if (LOG.isDebugEnabled())
LOG.debug("Non-resident session {} in cache", id);
return null;
}
else if (enter)
{
session.use();
}
}
}
if (ex != null)
throw ex;
return session;
}

41
jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java
View File

@ -19,6 +19,8 @@
package org.eclipse.jetty.server.session;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
@ -80,18 +82,12 @@ public class DefaultSessionCache extends AbstractSessionCache
return _stats.getTotal();
}
/**
*
*/
@ManagedOperation(value = "reset statistics", impact = "ACTION")
public void resetStats()
{
_stats.reset();
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doGet(java.lang.String)
*/
@Override
public Session doGet(String id)
{
@ -103,26 +99,32 @@ public class DefaultSessionCache extends AbstractSessionCache
return session;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doPutIfAbsent(java.lang.String, org.eclipse.jetty.server.session.Session)
*/
@Override
public Session doPutIfAbsent(String id, Session session)
{
Session s = _sessions.putIfAbsent(id, session);
if (s == null && !(session instanceof PlaceHolderSession))
if (s == null)
_stats.increment();
return s;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doDelete(java.lang.String)
*/
@Override
protected Session doComputeIfAbsent(String id, Function<String, Session> mappingFunction)
{
return _sessions.computeIfAbsent(id, k ->
{
Session s = mappingFunction.apply(k);
if (s != null)
_stats.increment();
return s;
});
}
@Override
public Session doDelete(String id)
{
Session s = _sessions.remove(id);
if (s != null && !(s instanceof PlaceHolderSession))
if (s != null)
_stats.decrement();
return s;
}
@ -168,9 +170,6 @@ public class DefaultSessionCache extends AbstractSessionCache
}
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(javax.servlet.http.HttpServletRequest, org.eclipse.jetty.server.session.SessionData)
*/
@Override
public Session newSession(HttpServletRequest request, SessionData data)
{
@ -178,9 +177,6 @@ public class DefaultSessionCache extends AbstractSessionCache
return s;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(org.eclipse.jetty.server.session.SessionData)
*/
@Override
public Session newSession(SessionData data)
{
@ -188,15 +184,10 @@ public class DefaultSessionCache extends AbstractSessionCache
return s;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doReplace(java.lang.String, org.eclipse.jetty.server.session.Session, org.eclipse.jetty.server.session.Session)
*/
@Override
public boolean doReplace(String id, Session oldValue, Session newValue)
{
boolean result = _sessions.replace(id, oldValue, newValue);
if (result && (oldValue instanceof PlaceHolderSession))
_stats.increment();
return result;
}
}

32
jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java
View File

@ -18,6 +18,8 @@
package org.eclipse.jetty.server.session;
import java.util.function.Function;
import javax.servlet.http.HttpServletRequest;
/**
@ -39,35 +41,23 @@ public class NullSessionCache extends AbstractSessionCache
super.setEvictionPolicy(EVICT_ON_SESSION_EXIT);
}
/**
* @see org.eclipse.jetty.server.session.SessionCache#shutdown()
*/
@Override
public void shutdown()
{
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(org.eclipse.jetty.server.session.SessionData)
*/
@Override
public Session newSession(SessionData data)
{
return new Session(getSessionHandler(), data);
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(javax.servlet.http.HttpServletRequest, org.eclipse.jetty.server.session.SessionData)
*/
@Override
public Session newSession(HttpServletRequest request, SessionData data)
{
return new Session(getSessionHandler(), request, data);
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doGet(java.lang.String)
*/
@Override
public Session doGet(String id)
{
@ -75,9 +65,6 @@ public class NullSessionCache extends AbstractSessionCache
return null;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doPutIfAbsent(java.lang.String, org.eclipse.jetty.server.session.Session)
*/
@Override
public Session doPutIfAbsent(String id, Session session)
{
@ -85,9 +72,6 @@ public class NullSessionCache extends AbstractSessionCache
return null;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doReplace(java.lang.String, org.eclipse.jetty.server.session.Session, org.eclipse.jetty.server.session.Session)
*/
@Override
public boolean doReplace(String id, Session oldValue, Session newValue)
{
@ -95,21 +79,21 @@ public class NullSessionCache extends AbstractSessionCache
return true;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#doDelete(java.lang.String)
*/
@Override
public Session doDelete(String id)
{
return null;
}
/**
* @see org.eclipse.jetty.server.session.AbstractSessionCache#setEvictionPolicy(int)
*/
@Override
public void setEvictionPolicy(int evictionTimeout)
{
LOG.warn("Ignoring eviction setting:" + evictionTimeout);
}
@Override
protected Session doComputeIfAbsent(String id, Function<String, Session> mappingFunction)
{
return mappingFunction.apply(id);
}
}

18
jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java
View File

@ -19,6 +19,8 @@
package org.eclipse.jetty.server.session;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import javax.servlet.SessionCookieConfig;
import javax.servlet.http.HttpServletRequest;
@ -35,10 +37,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class SessionCookieTest
{
public class MockSessionStore extends AbstractSessionCache
public class MockSessionCache extends AbstractSessionCache
{
public MockSessionStore(SessionHandler manager)
public MockSessionCache(SessionHandler manager)
{
super(manager);
}
@ -83,6 +85,12 @@ public class SessionCookieTest
{
return null;
}
@Override
protected Session doComputeIfAbsent(String id, Function<String, Session> mappingFunction)
{
return mappingFunction.apply(id);
}
}
public class MockSessionIdManager extends DefaultSessionIdManager
@ -118,9 +126,9 @@ public class SessionCookieTest
MockSessionIdManager idMgr = new MockSessionIdManager(server);
idMgr.setWorkerName("node1");
SessionHandler mgr = new SessionHandler();
MockSessionStore store = new MockSessionStore(mgr);
store.setSessionDataStore(new NullSessionDataStore());
mgr.setSessionCache(store);
MockSessionCache cache = new MockSessionCache(mgr);
cache.setSessionDataStore(new NullSessionDataStore());
mgr.setSessionCache(cache);
mgr.setSessionIdManager(idMgr);
long now = TimeUnit.NANOSECONDS.toMillis(System.nanoTime());

16
tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java
View File

@ -69,6 +69,7 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
DefaultSessionCacheFactory cacheFactory1 = new DefaultSessionCacheFactory();
cacheFactory1.setEvictionPolicy(SessionCache.NEVER_EVICT); //don't evict sessions
cacheFactory1.setFlushOnResponseCommit(true);
SessionDataStoreFactory storeFactory1 = createSessionDataStoreFactory();
((AbstractSessionDataStoreFactory)storeFactory1).setGracePeriodSec(scavengePeriod);
((AbstractSessionDataStoreFactory)storeFactory1).setSavePeriodSec(0); //always save when the session exits
@ -77,8 +78,6 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
TestServlet servlet1 = new TestServlet();
ServletHolder holder1 = new ServletHolder(servlet1);
ServletContextHandler context = server1.addContext(contextPath);
TestHttpChannelCompleteListener scopeListener = new TestHttpChannelCompleteListener();
server1.getServerConnector().addBean(scopeListener);
TestSessionListener listener1 = new TestSessionListener();
context.getSessionHandler().addEventListener(listener1);
context.addServlet(holder1, servletMapping);
@ -91,13 +90,12 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
DefaultSessionCacheFactory cacheFactory2 = new DefaultSessionCacheFactory();
cacheFactory2.setEvictionPolicy(SessionCache.NEVER_EVICT); //don't evict sessions
cacheFactory2.setFlushOnResponseCommit(true);
SessionDataStoreFactory storeFactory2 = createSessionDataStoreFactory();
((AbstractSessionDataStoreFactory)storeFactory2).setGracePeriodSec(scavengePeriod);
((AbstractSessionDataStoreFactory)storeFactory2).setSavePeriodSec(0); //always save when the session exits
TestServer server2 = new TestServer(0, maxInactivePeriod, scavengePeriod, cacheFactory2, storeFactory2);
ServletContextHandler context2 = server2.addContext(contextPath);
TestHttpChannelCompleteListener scopeListener2 = new TestHttpChannelCompleteListener();
server2.getServerConnector().addBean(scopeListener2);
context2.addServlet(TestServlet.class, servletMapping);
SessionHandler m2 = context2.getSessionHandler();
@ -110,18 +108,12 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
try
{
// Perform one request to server1 to create a session
CountDownLatch synchronizer = new CountDownLatch(1);
scopeListener.setExitSynchronizer(synchronizer);
ContentResponse response1 = client.GET("http://localhost:" + port1 + contextPath + servletMapping.substring(1) + "?action=init");
assertEquals(HttpServletResponse.SC_OK, response1.getStatus());
assertTrue(response1.getContentAsString().startsWith("init"));
String sessionCookie = response1.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
String id = TestServer.extractSessionId(sessionCookie);
//ensure request has finished being handled
synchronizer.await(5, TimeUnit.SECONDS);
assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsCurrent());
assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsMax());
assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsTotal());
@ -140,14 +132,10 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
long time = start;
while (time < end)
{
synchronizer = new CountDownLatch(1);
scopeListener2.setExitSynchronizer(synchronizer);
Request request = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping.substring(1));
ContentResponse response2 = request.send();
assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
assertTrue(response2.getContentAsString().startsWith("test"));
//ensure request has finished being handled
synchronizer.await(5, TimeUnit.SECONDS);
Thread.sleep(requestInterval);
assertSessionCounts(1, 1, 1, m2);
time = System.currentTimeMillis();