Issue #1386

jetty-gcloud/jetty-gcloud-session-manager/src/main/config-template/etc/sessions/gcloud/session-store.xml

@@ -11,6 +11,7 @@
    <Arg>
     <New id="sessionDataStoreFactory" class="org.eclipse.jetty.gcloud.session.GCloudSessionDataStoreFactory">      
        <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
        <Set name="maxRetries"><Property name="jetty.session.gcloud.maxRetries" default="5"/></Set>
        <Set name="backoffMs"><Property name="jetty.session.gcloud.backoffMs" default="1000"/></Set>
        <Set name="namespace"><Property name="jetty.session.gcloud.namespace" default=""/></Set>

jetty-gcloud/jetty-gcloud-session-manager/src/main/config-template/modules/session-store-gcloud.mod

@@ -26,6 +26,8 @@ etc/sessions/gcloud/session-store.xml
 [ini-template]
 
 ## GCloudDatastore Session config
+#jetty.session.gracePeriod.seconds=3600
+#jetty.session.savePeriod.seconds=0
 #jetty.session.gcloud.maxRetries=5
 #jetty.session.gcloud.backoffMs=1000
 #jetty.session.gcloud.namespace=

jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java

@@ -548,11 +548,15 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore
             for (ExpiryInfo item:info)
             {
                 if (StringUtil.isBlank(item.getLastNode()))
+                {
                     expired.add(item.getId()); //nobody managing it
+                }
                 else
                 {
                     if (_context.getWorkerName().equals(item.getLastNode()))
+                    {
                         expired.add(item.getId()); //we're managing it, we can expire it
+                    }
                     else
                     {
                         if (_lastExpiryCheckTime <= 0)
@@ -560,8 +564,7 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore
                             //our first check, just look for sessions that we managed by another node that
                             //expired at least 3 graceperiods ago
                             if (item.getExpiry() < (now - (1000L * (3 * _gracePeriodSec))))
-                                expired.add(item.getId());
+                                expired.add(item.getId());                        }
-                        }
                         else
                         {
                             //another node was last managing it, only expire it if it expired a graceperiod ago
@@ -655,11 +658,12 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore
      */
     protected Set<ExpiryInfo>  queryExpiryByIndex () throws Exception
     {
+        long now = System.currentTimeMillis();
         Set<ExpiryInfo> info = new HashSet<>();
         Query<ProjectionEntity> query = Query.newProjectionEntityQueryBuilder()
                 .setKind(_model.getKind())
                 .setProjection(_model.getId(), _model.getLastNode(), _model.getExpiry())
-                .setFilter(CompositeFilter.and(PropertyFilter.gt(_model.getExpiry(), 0), PropertyFilter.le(_model.getExpiry(), System.currentTimeMillis())))
+                .setFilter(CompositeFilter.and(PropertyFilter.gt(_model.getExpiry(), 0), PropertyFilter.le(_model.getExpiry(), now)))
                 .setLimit(_maxResults)
                 .build();
 
@@ -765,7 +769,6 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore
     public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
     {
         if (LOG.isDebugEnabled()) LOG.debug("Writing session {} to DataStore", data.getId());
-
         Entity entity = entityFromSession(data, makeKey(id, _context));
 
         //attempt the update with exponential back-off

jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreFactory.java

@@ -94,6 +94,7 @@ public class GCloudSessionDataStoreFactory extends AbstractSessionDataStoreFacto
         ds.setMaxRetries(getMaxRetries());
         ds.setGracePeriodSec(getGracePeriodSec());
         ds.setNamespace(_namespace);
+        ds.setSavePeriodSec(getSavePeriodSec());
         return ds;
     }
 

jetty-infinispan/src/main/config/etc/sessions/infinispan/default.xml

@@ -22,6 +22,7 @@
        <Set name="cache"><Ref id="cache"/></Set>
        <Set name="infinispanIdleTimeoutSec"><Property name="jetty.session.infinispan.idleTimeout.seconds" default="0" /></Set>
        <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
     </New>
    </Arg>
   </Call>

jetty-infinispan/src/main/config/etc/sessions/infinispan/remote.xml

@@ -24,6 +24,7 @@
        <Set name="cache"><Ref id="remoteCache"/></Set>
        <Set name="infinispanIdleTimeoutSec"><Property name="jetty.session.infinispan.idleTimeout.seconds" default="0" /></Set>
        <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
     </New>
    </Arg>
   </Call>

jetty-infinispan/src/main/config/modules/session-store-infinispan-embedded.mod

@@ -25,3 +25,6 @@ Infinispan is an open source project hosted on Github and released under the Apa
 http://infinispan.org/
 http://www.apache.org/licenses/LICENSE-2.0.html
 
+[ini-template]
+#jetty.session.gracePeriod.seconds=3600
+#jetty.session.savePeriod.seconds=0

jetty-infinispan/src/main/config/modules/session-store-infinispan-remote.mod

@@ -31,8 +31,4 @@ http://www.apache.org/licenses/LICENSE-2.0.html
 #jetty.session.infinispan.remoteCacheName=sessions
 #jetty.session.infinispan.idleTimeout.seconds=0
 #jetty.session.gracePeriod.seconds=3600
-
+#jetty.session.savePeriod.seconds=0
-
-
-
-

jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStoreFactory.java

@@ -61,6 +61,7 @@ public class InfinispanSessionDataStoreFactory extends AbstractSessionDataStoreF
         store.setGracePeriodSec(getGracePeriodSec());
         store.setInfinispanIdleTimeoutSec(getInfinispanIdleTimeoutSec());
         store.setCache(getCache());
+        store.setSavePeriodSec(getSavePeriodSec());
         return store;
     }
     

jetty-nosql/src/main/config/etc/sessions/mongo/session-store-by-address.xml

@@ -13,6 +13,7 @@
        <Set name="dbName"><Property name="jetty.session.mongo.dbName" default="HttpSessions" /></Set>
        <Set name="collectionName"><Property name="jetty.session.mongo.collectionName" default="jettySessions" /></Set>
        <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
        <Set name="host"><Property name="jetty.session.mongo.host" default="localhost"/></Set>
        <Set name="port"><Property name="jetty.session.mongo.port" default="27017"/></Set>
     </New>

jetty-nosql/src/main/config/etc/sessions/mongo/session-store-by-uri.xml

@@ -13,6 +13,7 @@
        <Set name="dbName"><Property name="jetty.session.mongo.dbName" default="HttpSessions" /></Set>
        <Set name="collectionName"><Property name="jetty.session.mongo.collectionName" default="jettySessions" /></Set>
        <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
        <Set name="connectionString"><Property name="jetty.session.mongo.connectionString" default="mongodb://localhost"/></Set>
     </New>
    </Arg>

jetty-nosql/src/main/config/modules/session-store-mongo.mod

@@ -31,6 +31,7 @@ connection-type?=address
 #jetty.session.mongo.dbName=HttpSessions
 #jetty.session.mongo.collectionName=jettySessions
 #jetty.session.gracePeriod.seconds=3600
+#jetty.session.savePeriod.seconds=0
 
 connection-type=address
 #jetty.session.mongo.host=localhost

jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreFactory.java

@@ -135,6 +135,7 @@ public class MongoSessionDataStoreFactory extends AbstractSessionDataStoreFactor
     {
         MongoSessionDataStore store = new MongoSessionDataStore();
         store.setGracePeriodSec(getGracePeriodSec());
+        store.setSavePeriodSec(getSavePeriodSec());
         Mongo mongo;
 
         if (!StringUtil.isBlank(getConnectionString()))

jetty-server/src/main/config/etc/sessions/file/session-store.xml

@@ -12,6 +12,7 @@
     <New id="sessionDataStoreFactory" class="org.eclipse.jetty.server.session.FileSessionDataStoreFactory">
        <Set name="deleteUnrestorableFiles"><Property name="jetty.session.file.deleteUnrestorableFiles" default="false" /></Set>
        <Set name="storeDir"><Property name="jetty.session.file.storeDir"/></Set>
+       <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
     </New>
    </Arg>
   </Call>

jetty-server/src/main/config/etc/sessions/jdbc/session-store.xml

@@ -11,6 +11,7 @@
    <Arg>
     <New id="sessionDataStoreFactory" class="org.eclipse.jetty.server.session.JDBCSessionDataStoreFactory">   
       <Set name="gracePeriodSec"><Property name="jetty.session.gracePeriod.seconds" default="3600" /></Set>
+      <Set name="savePeriodSec"><Property name="jetty.session.savePeriod.seconds" default="0" /></Set>
       <Set name="databaseAdaptor">
         <Ref id="databaseAdaptor"/>
       </Set>

jetty-server/src/main/config/modules/session-store-file.mod

@@ -19,4 +19,4 @@ sessions/
 [ini-template]
 jetty.session.file.storeDir=${jetty.base}/sessions
 #jetty.session.file.deleteUnrestorableFiles=false
-
+#jetty.session.savePeriod.seconds=0

jetty-server/src/main/config/modules/session-store-jdbc.mod

@@ -23,6 +23,7 @@ db-connection-type?=datasource
 ##
 
 #jetty.session.gracePeriod.seconds=3600
+#jetty.session.savePeriod.seconds=0
 
 ## Connection type:Datasource
 db-connection-type=datasource

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java

@@ -21,9 +21,12 @@ package org.eclipse.jetty.server.session;
 
 
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
 
 /**
  * AbstractSessionDataStore
@@ -32,10 +35,12 @@ import org.eclipse.jetty.util.component.ContainerLifeCycle;
  */
 public abstract class AbstractSessionDataStore extends ContainerLifeCycle implements SessionDataStore
 {
+    final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session");
+    
     protected SessionContext _context; //context associated with this session data store
     protected int _gracePeriodSec = 60 * 60; //default of 1hr 
     protected long _lastExpiryCheckTime = 0; //last time in ms that getExpired was called
-
+    protected int _savePeriodSec = 0; //time in sec between saves
 
     /**
      * Store the session data persistently.
@@ -74,21 +79,33 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem
     @Override
     public void store(String id, SessionData data) throws Exception
     {
+        if (data == null)
+            return;
+
+
         long lastSave = data.getLastSaved();
+        long savePeriodMs = (_savePeriodSec <=0? 0: TimeUnit.SECONDS.toMillis(_savePeriodSec));
         
-        //set the last saved time to now
+        if (LOG.isDebugEnabled())
-        data.setLastSaved(System.currentTimeMillis());
+            LOG.debug("Store: id={}, dirty={}, lsave={}, period={}, elapsed={}", id,data.isDirty(), data.getLastSaved(), savePeriodMs, (System.currentTimeMillis()-lastSave));
-        try
+
+        //save session if attribute changed or never been saved or time between saves exceeds threshold
+        if (data.isDirty() || (lastSave <= 0) || ((System.currentTimeMillis()-lastSave) > savePeriodMs))
         {
-            //call the specific store method, passing in previous save time
+            //set the last saved time to now
-            doStore(id, data, lastSave);
+            data.setLastSaved(System.currentTimeMillis());
-            data.setDirty(false); //only undo the dirty setting if we saved it
+            try
-        }
+            {
-        catch (Exception e)
+                //call the specific store method, passing in previous save time
-        {
+                doStore(id, data, lastSave);
-            //reset last save time if save failed
+                data.setDirty(false); //only undo the dirty setting if we saved it
-            data.setLastSaved(lastSave);
+            }
-            throw e;
+            catch (Exception e)
+            {
+                //reset last save time if save failed
+                data.setLastSaved(lastSave);
+                throw e;
+            }
         }
     }
     
@@ -148,6 +165,37 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem
     }
 
 
+    /**
+     * @return the savePeriodSec
+     */
+    public int getSavePeriodSec()
+    {
+        return _savePeriodSec;
+    }
+
+
+    /** 
+     * The minimum time in seconds between save operations.
+     * Saves normally occur every time the last request 
+     * exits as session. If nothing changes on the session
+     * except for the access time and the persistence technology
+     * is slow, this can cause delays.
+     * <p>
+     * By default the value is 0, which means we save 
+     * after the last request exists. A non zero value
+     * means that we will skip doing the save if the
+     * session isn't dirty if the elapsed time since
+     * the session was last saved does not exceed this
+     * value.
+     * 
+     * @param savePeriodSec the savePeriodSec to set
+     */
+    public void setSavePeriodSec(int savePeriodSec)
+    {
+        _savePeriodSec = savePeriodSec;
+    }
+
+
     /** 
      * @see java.lang.Object#toString()
      */

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreFactory.java

@@ -28,6 +28,7 @@ public abstract class AbstractSessionDataStoreFactory implements SessionDataStor
 {
 
     int _gracePeriodSec;
+    int _savePeriodSec;
     
     
     
@@ -47,6 +48,24 @@ public abstract class AbstractSessionDataStoreFactory implements SessionDataStor
     {
         _gracePeriodSec = gracePeriodSec;
     }
+
+
+    /**
+     * @return the savePeriodSec
+     */
+    public int getSavePeriodSec()
+    {
+        return _savePeriodSec;
+    }
+
+
+    /**
+     * @param savePeriodSec the savePeriodSec to set
+     */
+    public void setSavePeriodSec(int savePeriodSec)
+    {
+        _savePeriodSec = savePeriodSec;
+    }
    
 
 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java

@@ -153,10 +153,14 @@ public class CachingSessionDataStore extends ContainerLifeCycle implements Sessi
     @Override
     public void store(String id, SessionData data) throws Exception
     {
+        long lastSaved = data.getLastSaved();
+        
         //write to the SessionDataStore first
         _store.store(id, data);
-        //then update the cache with written data
+
-        _cache.store(id,data);
+        //if the store saved it, then update the cache too
+        if (data.getLastSaved() != lastSaved)
+            _cache.store(id,data); 
     }
 
   

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

@@ -145,20 +145,17 @@ public class DefaultSessionCache extends AbstractSessionCache
         {
             for (Session session: _sessions.values())
             {
-                //if we have a backing store and the session is dirty make sure it is written out
+                //if we have a backing store so give the session to it to write out if necessary
                 if (_sessionDataStore != null)
                 {
-                    if (session.getSessionData().isDirty())
+                    session.willPassivate();
+                    try
                     {
-                        session.willPassivate();
+                        _sessionDataStore.store(session.getId(), session.getSessionData());
-                        try
+                    }
-                        {
+                    catch (Exception e)
-                            _sessionDataStore.store(session.getId(), session.getSessionData());
+                    {
-                        }
+                        LOG.warn(e);
-                        catch (Exception e)
-                        {
-                            LOG.warn(e);
-                        }
                     }
                     doDelete (session.getId()); //remove from memory
                 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java

@@ -35,9 +35,11 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.StringTokenizer;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.jetty.util.ClassLoadingObjectInputStream;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
@@ -120,9 +122,42 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         final long now = System.currentTimeMillis();
         HashSet<String> expired = new HashSet<String>();
         
-        File[] files = _storeDir.listFiles(new FilenameFilter() 
+        HashSet<String> idsWithContext = new HashSet<>();
-        {
+        
+        
+        //one pass to get all idWithContext
+        File [] files = _storeDir.listFiles(new FilenameFilter()
+        {  
+            @Override
+            public boolean accept(File dir, String name)
+            {
+                if (dir != _storeDir)
+                    return false;
 
+                //dir may contain files that don't match our naming pattern      
+                if (!match(name))
+                {
+                    return false;
+                }
+
+                String idWithContext = getIdWithContextFromString(name);
+                if (!StringUtil.isBlank(idWithContext))
+                    idsWithContext.add(idWithContext);
+                return true;
+            }
+        });
+        
+       
+        //got the list of all sessionids with their contexts, remove all old files for each one
+        for (String idWithContext:idsWithContext)
+        {
+            deleteOldFiles(_storeDir, idWithContext);
+        }
+
+        
+        //now find sessions that have expired in any context
+        files = _storeDir.listFiles(new FilenameFilter() 
+        {      
             @Override
             public boolean accept(File dir, String name)
             {
@@ -130,16 +165,15 @@ public class FileSessionDataStore extends AbstractSessionDataStore
                     return false;
 
                 //dir may contain files that don't match our naming pattern
-                int index = name.indexOf('_');
+                if (!match(name))
-                if (index < 0)
                     return false;
-
+                
                 try
                 {
-                    long expiry = Long.parseLong(name.substring(0, index));
+                    long expiry = getExpiryFromString(name);
                     return expiry > 0 && expiry < now;
                 }
-                catch (NumberFormatException e)
+                catch (Exception e)
                 {
                     return false;
                 }
@@ -184,8 +218,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         {
             public void run ()
             {
-                File file = getFile(_storeDir,id);
+                //get rid of all but the newest file for a session
-
+                File file = deleteOldFiles(_storeDir, getIdWithContext(id));
+   
                 if (file == null || !file.exists())
                 {
                     if (LOG.isDebugEnabled())
@@ -197,17 +232,16 @@ public class FileSessionDataStore extends AbstractSessionDataStore
                 {
                     SessionData data = load(in);
                     data.setLastSaved(file.lastModified());
-                    //delete restored file
-                    file.delete();
                     reference.set(data);
                 }
                 catch (UnreadableSessionDataException e)
                 {
-                    if (isDeleteUnrestorableFiles() && file.exists() && file.getParentFile().equals(_storeDir));
+                    if (isDeleteUnrestorableFiles() && file.exists() && file.getParentFile().equals(_storeDir))
                     {
                         file.delete();
                         LOG.warn("Deleted unrestorable file for session {}", id);
                     }
+
                     exception.set(e);
                 }
                 catch (Exception e)
@@ -236,13 +270,11 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         File file = null;
         if (_storeDir != null)
         {
-            //remove any existing file for the session
+            //remove any existing files for the session
-            file = getFile(_storeDir, id);
+            deleteAllFiles(_storeDir, getIdWithContext(id));
-            if (file != null && file.exists())
-                file.delete();
             
             //make a fresh file using the latest session expiry
-            file = new File(_storeDir, getFileNameWithExpiry(data));
+            file = new File(_storeDir, getIdWithContextAndExpiry(data));
 
             try(FileOutputStream fos = new FileOutputStream(file,false))
             {
@@ -288,7 +320,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     @Override
     public boolean exists(String id) throws Exception
     {
-       File sessionFile = getFile(_storeDir, id);
+       File sessionFile = deleteOldFiles(_storeDir, getIdWithContext(id));
        if (sessionFile == null || !sessionFile.exists())
            return false;
        
@@ -332,19 +364,32 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     }
 
     /**
+     * Get the session id with its context.
+     * 
      * @param id identity of session
-     * @return the filename of the session data store
+     * @return the session id plus context
      */
-    private String getFileName (String id)
+    private String getIdWithContext (String id)
     {
         return _context.getCanonicalContextPath()+"_"+_context.getVhost()+"_"+id;
     }
     
-    private String getFileNameWithExpiry (SessionData data)
+    /**
+     * Get the session id with its context and its expiry time
+     * @param data
+     * @return the session id plus context and expiry
+     */
+    private String getIdWithContextAndExpiry (SessionData data)
     {
-        return ""+data.getExpiry()+"_"+getFileName(data.getId());
+        return ""+data.getExpiry()+"_"+getIdWithContext(data.getId());
     }
     
+    
+    /**
+     * Work out which session id the file relates to.
+     * @param file the file to check
+     * @return the session id the file relates to.
+     */
     private String getIdFromFile (File file)
     {
         if (file == null)
@@ -354,15 +399,73 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         return name.substring(name.lastIndexOf('_')+1);
     }
     
+    /**
+     * Get the expiry time of the session stored in the file.
+     * @param file the file from which to extract the expiry time
+     * @return the expiry time
+     */
     private long getExpiryFromFile (File file)
     {
         if (file == null)
             return 0;
         
-        String name = file.getName();
+        return getExpiryFromString(file.getName());
-        String s = name.substring(0, name.indexOf('_'));
+    }
+    
+    
+    private long getExpiryFromString (String filename)
+    {
+        if (StringUtil.isBlank(filename) || filename.indexOf("_") < 0)
+            throw new IllegalStateException ("Invalid or missing filename");
+        
+        String s = filename.substring(0, filename.indexOf('_'));
         return (s==null?0:Long.parseLong(s));
     }
+    
+    /**
+     * Extract the session id and context from the filename.
+     * @param file the file whose name to use
+     * @return the session id plus context
+     */
+    private String getIdWithContextFromFile (File file)
+    {
+        if (file == null)
+            return null;
+        
+        String s =  getIdWithContextFromString(file.getName());
+        return s;
+    }
+    
+    /**
+     * Extract the session id and context from the filename
+     * @param filename the name of the file to use
+     * @return the session id plus context
+     */
+    private String getIdWithContextFromString (String filename)
+    {
+        if (StringUtil.isBlank(filename) || filename.indexOf('_') < 0)
+            return null;
+        
+        return filename.substring(filename.indexOf('_')+1);
+    }
+    
+    /**
+     * Check if the filename matches our session pattern
+     * @param filename
+     * @return
+     */
+    private boolean match (String filename)
+    {
+        if (StringUtil.isBlank(filename))
+            return false;
+        String[] parts = filename.split("_");
+        
+        //Need at least 4 parts for a valid filename
+        if (parts.length < 4)
+            return false;
+        
+        return true;
+    }
 
     
     /**
@@ -384,7 +487,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             {
                 if (dir != storeDir)
                     return false;
-                return (name.contains(getFileName(id)));
+                return (name.contains(getIdWithContext(id)));
             }
             
         });
@@ -393,6 +496,97 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             return null;
         return files[0];
     }
+    
+    
+    private void deleteAllFiles(final File storeDir, final String idInContext)
+    {
+        File[] files = storeDir.listFiles (new FilenameFilter() {
+
+            /** 
+             * @see java.io.FilenameFilter#accept(java.io.File, java.lang.String)
+             */
+            @Override
+            public boolean accept(File dir, String name)
+            {
+                if (dir != storeDir)
+                    return false;
+                return (name.contains(idInContext));
+            }
+            
+        });
+        
+        //no files for that id
+        if (files == null || files.length < 1)
+            return;
+        
+        //delete all files
+        for (File f:files)
+        {
+           f.delete();
+        }
+    }
+    
+    
+    
+    /**
+     * Delete all but the most recent file for a given session id in a context.
+     * 
+     * @param storeDir the directory in which sessions are stored
+     * @param idWithContext the id of the session
+     * @return the most recent remaining file for the session, can be null
+     */
+    private File deleteOldFiles (final File storeDir, final String idWithContext)
+    {
+        File[] files = storeDir.listFiles (new FilenameFilter() {
+
+            /** 
+             * @see java.io.FilenameFilter#accept(java.io.File, java.lang.String)
+             */
+            @Override
+            public boolean accept(File dir, String name)
+            {
+                if (dir != storeDir)
+                    return false;
+                
+                if (!match(name))
+                    return false;
+                
+                return (name.contains(idWithContext));
+            }
+            
+        });
+        
+        //no file for that session
+        if (files == null || files.length == 0)
+            return null;
+
+        
+        //delete all but the most recent file
+        File file = null;
+        for (File f:files)
+        {
+            if (file == null)
+                file = f;
+            else
+            {
+               //accept the newest file
+                if (f.lastModified() > file.lastModified())
+                {
+                    file.delete();
+                    file = f;
+                }
+                else
+                {
+                    f.delete();
+                }
+            }
+        }
+
+        return file;
+    }
+    
+    
+    
 
     /**
      * @param is inputstream containing session data

jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStoreFactory.java

@@ -78,6 +78,7 @@ public class FileSessionDataStoreFactory extends AbstractSessionDataStoreFactory
         fsds.setDeleteUnrestorableFiles(isDeleteUnrestorableFiles());
         fsds.setStoreDir(getStoreDir());
         fsds.setGracePeriodSec(getGracePeriodSec());
+        fsds.setSavePeriodSec(getSavePeriodSec());
         return fsds;
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreFactory.java

@@ -48,6 +48,7 @@ public class JDBCSessionDataStoreFactory extends AbstractSessionDataStoreFactory
         ds.setDatabaseAdaptor(_adaptor);
         ds.setSessionTableSchema(_schema);
         ds.setGracePeriodSec(getGracePeriodSec());
+        ds.setSavePeriodSec(getSavePeriodSec());
         return ds;
     }
 

jetty-server/src/test/java/org/eclipse/jetty/server/session/FileSessionManagerTest.java

@@ -29,6 +29,7 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.StdErrLog;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -54,10 +55,63 @@ public class FileSessionManagerTest
         _log.setHideStacks(_stacks);
     }
     
+    @After
+    public void after()
+    {
+        File testDir = MavenTestingUtils.getTargetTestingDir("hashes");
+        if (testDir.exists())
+            FS.ensureEmpty(testDir);
+    }
     
     
     @Test
     public void testDangerousSessionIdRemoval() throws Exception
+    {  
+        String expectedFilename =  "_0.0.0.0_dangerFile";    
+        File targetFile = MavenTestingUtils.getTargetFile(expectedFilename);
+
+        try
+        {
+            Server server = new Server();
+            SessionHandler handler = new SessionHandler();
+            handler.setServer(server);
+            final DefaultSessionIdManager idmgr = new DefaultSessionIdManager(server);
+            idmgr.setServer(server);
+            server.setSessionIdManager(idmgr);
+
+            FileSessionDataStore ds = new FileSessionDataStore();
+            ds.setDeleteUnrestorableFiles(true);
+            DefaultSessionCache ss = new DefaultSessionCache(handler);
+            handler.setSessionCache(ss);
+            ss.setSessionDataStore(ds);
+            File testDir = MavenTestingUtils.getTargetTestingDir("hashes");
+            testDir.mkdirs();
+            ds.setStoreDir(testDir);
+            handler.setSessionIdManager(idmgr);
+            handler.start();
+
+            //Create a file that is in the parent dir of the session storeDir
+
+            targetFile.createNewFile();
+            Assert.assertTrue("File should exist!", MavenTestingUtils.getTargetFile(expectedFilename).exists());
+
+            //Verify that passing in a relative filename outside of the storedir does not lead
+            //to deletion of file (needs deleteUnrecoverableFiles(true))
+            Session session = handler.getSession("../_0.0.0.0_dangerFile");
+            Assert.assertTrue(session == null);
+            Assert.assertTrue("File should exist!", MavenTestingUtils.getTargetFile(expectedFilename).exists());
+        }
+        finally
+        {
+            if (targetFile.exists())
+                IO.delete(targetFile);
+        }
+    }
+    
+    
+    
+    @Test
+    public void testDeleteOfOlderFiles() throws Exception
     {
         Server server = new Server();
         SessionHandler handler = new SessionHandler();
@@ -67,32 +121,51 @@ public class FileSessionManagerTest
         server.setSessionIdManager(idmgr);
         
         FileSessionDataStore ds = new FileSessionDataStore();
-        ds.setDeleteUnrestorableFiles(true);
+        ds.setDeleteUnrestorableFiles(false); //turn off deletion of unreadable session files
         DefaultSessionCache ss = new DefaultSessionCache(handler);
         handler.setSessionCache(ss);
         ss.setSessionDataStore(ds);
-        //manager.setLazyLoad(true);
         File testDir = MavenTestingUtils.getTargetTestingDir("hashes");
         testDir.mkdirs();
         ds.setStoreDir(testDir);
         handler.setSessionIdManager(idmgr);
         handler.start();
+
+        //create a bunch of older files for same session abc
+        String name1 =  "100__0.0.0.0_abc";    
+        File f1 = new File(testDir, name1);
+        if (f1.exists())
+            f1.delete();
+        f1.createNewFile();
+
+        Thread.currentThread().sleep(20);
         
-        //Create a file that is in the parent dir of the session storeDir
+        String name2 = "101__0.0.0.0_abc"; 
-        String expectedFilename =  "_0.0.0.0_dangerFile";    
+        File f2 = new File(testDir, name2);
-        MavenTestingUtils.getTargetFile(expectedFilename).createNewFile();
+        if (f2.exists())
-        Assert.assertTrue("File should exist!", MavenTestingUtils.getTargetFile(expectedFilename).exists());
+            f2.delete();
-
+        f2.createNewFile();
-        //Verify that passing in the relative filename of an unrecoverable session does not lead
+        
-        //to deletion of file outside the session dir (needs deleteUnrecoverableFiles(true))
+        Thread.currentThread().sleep(20);
-        Session session = handler.getSession("../_0.0.0.0_dangerFile");
+        
-        Assert.assertTrue(session == null);
+        String name3 = "102__0.0.0.0_abc";
-        Assert.assertTrue("File should exist!", MavenTestingUtils.getTargetFile(expectedFilename).exists());
+        File f3 = new File(testDir, name3);
+        if (f3.exists())
+            f3.delete();       
+        f3.createNewFile();
 
+        Thread.currentThread().sleep(20);
+        
+        Session session = handler.getSession("abc");
+        Assert.assertTrue(!f1.exists()); 
+        Assert.assertTrue(!f2.exists());
+        Assert.assertTrue(f3.exists());
     }
 
+    
+    
     @Test
-    public void testValidSessionIdRemoval() throws Exception
+    public void testUnrestorableFileRemoval() throws Exception
     {      
         Server server = new Server();
         SessionHandler handler = new SessionHandler();
@@ -105,7 +178,7 @@ public class FileSessionManagerTest
         FileSessionDataStore ds = new FileSessionDataStore();
         ss.setSessionDataStore(ds);
         handler.setSessionCache(ss);
-        ds.setDeleteUnrestorableFiles(true);
+        ds.setDeleteUnrestorableFiles(true); //invalid file will be removed
         handler.setSessionIdManager(idmgr);
       
         File testDir = MavenTestingUtils.getTargetTestingDir("hashes");
@@ -114,7 +187,7 @@ public class FileSessionManagerTest
         ds.setStoreDir(testDir);
         handler.start();
 
-        String expectedFilename = "_0.0.0.0_validFile123";
+        String expectedFilename = (System.currentTimeMillis()+ 10000)+"__0.0.0.0_validFile123";
         
         Assert.assertTrue(new File(testDir, expectedFilename).createNewFile());
 
@@ -212,7 +285,6 @@ public class FileSessionManagerTest
         DefaultSessionCache ss = new DefaultSessionCache(handler);
         handler.setSessionCache(ss);
         ss.setSessionDataStore(ds);
-        //manager.setLazyLoad(true);
         File testDir = MavenTestingUtils.getTargetTestingDir("hashes");
         testDir.mkdirs();
         ds.setStoreDir(testDir);

tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java

@@ -71,6 +71,26 @@ public class FileTestHelper
         }
     }
 
+    
+    public static File getFile (String sessionId)
+    {
+        assertNotNull(_tmpDir);
+        assertTrue(_tmpDir.exists());
+        String[] files = _tmpDir.list();
+        assertNotNull(files);
+        String fname = null;
+        for (String name:files)
+        {
+            if (name.contains(sessionId))
+            {
+                fname=name;
+                break;
+            }
+        }
+        if (fname != null)
+            return new File (_tmpDir, fname);
+        return null;
+    }
 
     public static void assertFileExists (String sessionId, boolean exists)
     {

tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/NonClusteredSessionScavengingTest.java

@@ -30,7 +30,6 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
     @Before
     public void before() throws Exception
     {
-        System.setProperty("org.eclipse.jetty.server.session.LEVEL", "DEBUG");
        FileTestHelper.setup();
     }
     
@@ -42,6 +41,16 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
     
    
 
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        FileTestHelper.assertFileExists(id, exists);
+        
+    }
+
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
      */

tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/TestFileSessions.java

@@ -0,0 +1,170 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+
+package org.eclipse.jetty.server.session;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.api.Request;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * TestFileSessions
+ *
+ *
+ */
+public class TestFileSessions extends AbstractTestBase
+{
+    @Before
+    public void before() throws Exception
+    {
+       FileTestHelper.setup();
+    }
+    
+    @After 
+    public void after()
+    {
+       FileTestHelper.teardown();
+    }
+ 
+
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
+     */
+    @Override
+    public SessionDataStoreFactory createSessionDataStoreFactory()
+    {
+        return FileTestHelper.newSessionDataStoreFactory();
+    }
+  
+    @Test
+    public void test () throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 5;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        SessionDataStoreFactory storeFactory = createSessionDataStoreFactory();
+        TestServer server1 = new TestServer(0, inactivePeriod, 2, cacheFactory, storeFactory);
+        server1.addContext(contextPath).addServlet(TestServlet.class, servletMapping);
+        try
+        {
+            server1.start();
+            int port1 = server1.getPort();
+            
+            HttpClient client = new HttpClient();
+            client.start();
+            
+            try
+            {
+                // Connect to server1 to create a session and get its session cookie
+                ContentResponse response1 = client.GET("http://localhost:" + port1 + contextPath + servletMapping + "?action=init");
+                assertEquals(HttpServletResponse.SC_OK,response1.getStatus());
+                String sessionCookie = response1.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                // Mangle the cookie, replacing Path with $Path, etc.
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+                
+                //check that the file for the session exists after creating the session
+                FileTestHelper.assertFileExists(TestServer.extractSessionId(sessionCookie), true);
+                File file1 = FileTestHelper.getFile(TestServer.extractSessionId(sessionCookie));
+                
+                
+                //request the session and check that the file for the session exists with an updated lastmodify
+                Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check");
+                request.header("Cookie", sessionCookie);
+                ContentResponse response2 = request.send();
+                assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
+                FileTestHelper.assertFileExists(TestServer.extractSessionId(sessionCookie), true);
+                File file2 = FileTestHelper.getFile(TestServer.extractSessionId(sessionCookie));
+                assertTrue (!file1.equals(file2));
+                assertTrue (file2.lastModified() > file1.lastModified());
+                
+                //invalidate the session and verify that the session file is deleted
+                request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=remove");
+                request.header("Cookie", sessionCookie);
+                response2 = request.send();
+                assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
+                FileTestHelper.assertFileExists(TestServer.extractSessionId(sessionCookie), false);
+                
+                //make another session
+                response1 = client.GET("http://localhost:" + port1 + contextPath + servletMapping + "?action=init");
+                assertEquals(HttpServletResponse.SC_OK,response1.getStatus());
+                sessionCookie = response1.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+                FileTestHelper.assertFileExists(TestServer.extractSessionId(sessionCookie), true);
+                
+                //wait for it to be scavenged
+                Thread.currentThread().sleep((inactivePeriod + 2)*1000);
+                FileTestHelper.assertFileExists(TestServer.extractSessionId(sessionCookie), false);
+                
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            server1.stop();
+        }
+    }
+    
+    public static class TestServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
+        {
+            String action = request.getParameter("action");
+            if ("init".equals(action))
+            {
+                HttpSession session = request.getSession(true);
+                session.setAttribute("A", "A");
+            }
+            else if ("remove".equals(action))
+            {
+                HttpSession session = request.getSession(false);
+                session.invalidate();
+                //assertTrue(session == null);
+            }
+            else if ("check".equals(action))
+            {
+                HttpSession session = request.getSession(false);
+            }
+        }
+    }
+    
+    
+}

tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/ClusteredOrphanedSessionTest.java

@@ -55,10 +55,5 @@ public class ClusteredOrphanedSessionTest extends AbstractClusteredOrphanedSessi
     public void testOrphanedSession() throws Exception
     {
         super.testOrphanedSession();
-        GCloudTestSuite.__testSupport.assertSessions(0);
     }
-    
-    
-    
-
 }

tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTestSupport.java

@@ -66,7 +66,8 @@ public class GCloudSessionTestSupport
         @Override
         public SessionDataStore getSessionDataStore(SessionHandler handler) throws Exception
         {
-            GCloudSessionDataStore ds = new GCloudSessionDataStore();
+            GCloudSessionDataStore ds = (GCloudSessionDataStore)super.getSessionDataStore(handler);
+            ds.setMaxRetries(GCloudSessionDataStore.DEFAULT_MAX_RETRIES);
             ds.setDatastore(_d);
             return ds;
         }

tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/NonClusteredSessionScavengingTest.java

@@ -19,6 +19,12 @@
 
 package org.eclipse.jetty.gcloud.session;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.Set;
+
 import org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest;
 import org.eclipse.jetty.server.session.SessionDataStoreFactory;
 import org.junit.After;
@@ -41,6 +47,29 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
     
     
 
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        try
+        {
+            Set<String> ids = GCloudTestSuite.__testSupport.getSessionIds();
+            if (exists)
+                assertTrue(ids.contains(id));
+            else
+                assertFalse(ids.contains(id));
+        }
+        catch (Exception e)
+        {
+            fail(e.getMessage());
+        }
+
+    }
+
+
+
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
      */

tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/NonClusteredSessionScavengingTest.java

@@ -28,6 +28,16 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
 
  
 
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        //noop, as we do not have a session store
+        
+    }
+
     @Test
     public void testNewSession() throws Exception
     {

tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredOrphanedSessionTest.java

@@ -0,0 +1,63 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+
+package org.eclipse.jetty.server.session;
+
+import org.eclipse.jetty.session.infinispan.InfinispanSessionDataStoreFactory;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * ClusteredOrphanedSessionTest
+ *
+ *
+ */
+public class ClusteredOrphanedSessionTest extends AbstractClusteredOrphanedSessionTest
+{
+    public static InfinispanTestSupport __testSupport;
+    
+    
+    @BeforeClass
+    public static void setup () throws Exception
+    {
+        __testSupport = new InfinispanTestSupport();
+        __testSupport.setup();
+    }
+    
+    @AfterClass
+    public static void teardown () throws Exception
+    {
+        __testSupport.teardown();
+    }
+    
+
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
+     */
+    @Override
+    public SessionDataStoreFactory createSessionDataStoreFactory()
+    {
+        InfinispanSessionDataStoreFactory factory = new InfinispanSessionDataStoreFactory();
+        factory.setCache(__testSupport.getCache());
+        return factory;
+    }
+  
+ 
+
+}

tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/NonClusteredSessionScavengingTest.java

@@ -19,6 +19,13 @@
 
 package org.eclipse.jetty.server.session;
 
+import static org.junit.Assert.fail;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+
+
 import org.eclipse.jetty.session.infinispan.InfinispanSessionDataStoreFactory;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -49,6 +56,28 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
     
 
 
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        assertNotNull(_dataStore);
+        
+        try
+        {
+            boolean inmap = _dataStore.exists(id);
+            if (exists)
+                assertTrue(inmap);
+            else
+                assertFalse(inmap);
+        }
+        catch (Exception e)
+        {
+            fail(e.getMessage());
+        }
+    }
+
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
      */

tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/NonClusteredSessionScavengingTest.java

@@ -19,6 +19,14 @@
 
 package org.eclipse.jetty.server.session.remote;
 
+
+import static org.junit.Assert.fail;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+
+
 import org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest;
 import org.eclipse.jetty.server.session.SessionDataStoreFactory;
 import org.eclipse.jetty.session.infinispan.InfinispanSessionDataStoreFactory;
@@ -49,6 +57,30 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
         __testSupport.teardown();
     }
 
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        assertNotNull(_dataStore);
+        
+        try
+        {
+            boolean inmap = _dataStore.exists(id);
+            if (exists)
+                assertTrue(inmap);
+            else
+                assertFalse(inmap);
+        }
+        catch (Exception e)
+        {
+            fail(e.getMessage());
+        }
+    }
+    
+    
+    
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
      */

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/NonClusteredSessionScavengingTest.java

@@ -18,9 +18,16 @@
 
 package org.eclipse.jetty.server.session;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+
+
 import org.junit.After;
 import org.junit.Test;
 
+
 /**
  * NonClusteredSessionScavengingTest
  */
@@ -37,6 +44,27 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
     }
 
     
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+        try
+        {
+            boolean inDb = JdbcTestHelper.existsInSessionTable(id, false);
+            if (exists)
+                assertTrue(inDb);
+            else
+                assertFalse(inDb);
+        }
+        catch (Exception e)
+        {
+            fail(e.getMessage());
+        }
+    }
+
+
     @After
     public void tearDown() throws Exception 
     {

tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/NonClusteredSessionScavengingTest.java

@@ -18,6 +18,12 @@
 
 package org.eclipse.jetty.nosql.mongodb;
 
+import static org.junit.Assert.fail;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+
 import org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest;
 import org.eclipse.jetty.server.session.SessionDataStoreFactory;
 import org.junit.AfterClass;
@@ -42,6 +48,31 @@ public class NonClusteredSessionScavengingTest extends AbstractNonClusteredSessi
         MongoTestHelper.dropCollection();
     }
     
+    
+    
+    
+    
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractNonClusteredSessionScavengingTest#assertSession(java.lang.String, boolean)
+     */
+    @Override
+    public void assertSession(String id, boolean exists)
+    {
+       assertNotNull(_dataStore);
+       try
+       {
+           boolean inmap = _dataStore.exists(id);
+           if (exists)
+               assertTrue(inmap);
+           else
+               assertFalse(inmap);
+       }
+       catch (Exception e)
+       {
+           fail(e.getMessage());
+       }
+    }
+
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory()
      */

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredOrphanedSessionTest.java

@@ -58,6 +58,10 @@ public abstract class AbstractClusteredOrphanedSessionTest extends AbstractTestB
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
         SessionDataStoreFactory storeFactory = createSessionDataStoreFactory();
+        if (storeFactory instanceof AbstractSessionDataStoreFactory)
+        {
+            ((AbstractSessionDataStoreFactory)storeFactory).setGracePeriodSec(0);
+        }
         
         TestServer server1 = new TestServer(0, inactivePeriod, -1, cacheFactory, storeFactory);
         server1.addContext(contextPath).addServlet(TestServlet.class, servletMapping);
@@ -68,7 +72,7 @@ public abstract class AbstractClusteredOrphanedSessionTest extends AbstractTestB
             int scavengePeriod = 2;
             
             DefaultSessionCacheFactory evictCacheFactory = new DefaultSessionCacheFactory();
-            cacheFactory.setEvictionPolicy(2);//evict after idle for 2 sec
+          //  cacheFactory.setEvictionPolicy(2);//evict after idle for 2 sec
             
             TestServer server2 = new TestServer(0, inactivePeriod, scavengePeriod, evictCacheFactory, storeFactory);
             server2.addContext(contextPath).addServlet(TestServlet.class, servletMapping);         

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractNonClusteredSessionScavengingTest.java

@@ -46,6 +46,11 @@ import org.junit.Test;
  */
 public abstract class AbstractNonClusteredSessionScavengingTest extends AbstractTestBase
 {
+    
+    public SessionDataStore _dataStore;
+    
+    public abstract void assertSession (String id, boolean exists);
+    
 
     public void pause(int scavenge)
     {
@@ -74,6 +79,8 @@ public abstract class AbstractNonClusteredSessionScavengingTest extends Abstract
         TestServer server = new TestServer(0, maxInactivePeriod, scavengePeriod,
                                                            cacheFactory, storeFactory);
         ServletContextHandler context = server.addContext("/");
+        _dataStore = context.getSessionHandler().getSessionCache().getSessionDataStore();
+        
         context.addServlet(TestServlet.class, servletMapping);
         String contextPath = "";
 
@@ -94,6 +101,8 @@ public abstract class AbstractNonClusteredSessionScavengingTest extends Abstract
 
                 // Let's wait for the scavenger to run
                 pause(maxInactivePeriod + scavengePeriod);
+                
+                assertSession (TestServer.extractSessionId(sessionCookie), false);
 
                 // The session should not be there anymore, but we present an old cookie
                 // The server should create a new session.
@@ -128,6 +137,7 @@ public abstract class AbstractNonClusteredSessionScavengingTest extends Abstract
         TestServer server = new TestServer(0, maxInactivePeriod, scavengePeriod,
                                                            cacheFactory, storeFactory);
         ServletContextHandler context = server.addContext("/");
+        _dataStore = context.getSessionHandler().getSessionCache().getSessionDataStore();
         context.addServlet(TestServlet.class, servletMapping);
         String contextPath = "";
 
@@ -149,6 +159,8 @@ public abstract class AbstractNonClusteredSessionScavengingTest extends Abstract
 
                 // Let's wait for the scavenger to run
                 pause(2*scavengePeriod);
+                
+                assertSession(TestServer.extractSessionId(sessionCookie), true);
 
                 // Test that the session is still there
                 Request request = client.newRequest("http://localhost:" + port + contextPath + servletMapping + "?action=old-test");

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestSessionDataStore.java

@@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session;
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -91,7 +92,18 @@ public class TestSessionDataStore extends AbstractSessionDataStore
     @Override
     public Set<String> doGetExpired(Set<String> candidates)
     {
-        return Collections.emptySet();
+       HashSet<String> set = new HashSet<>();
+        long now = System.currentTimeMillis();
+        
+       
+        for (SessionData d:_map.values())
+        {
+            if (d.getExpiry() > 0 && d.getExpiry() <= now)
+                set.add(d.getId());
+        }
+        return set;
+        
+        //return Collections.emptySet();
     }
 
 }

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestSessionDataStoreFactory.java

@@ -33,7 +33,9 @@ public class TestSessionDataStoreFactory extends AbstractSessionDataStoreFactory
     @Override
     public SessionDataStore getSessionDataStore(SessionHandler handler) throws Exception
     {
-       return new TestSessionDataStore();
+       TestSessionDataStore store = new TestSessionDataStore();
+       store.setSavePeriodSec(getSavePeriodSec());
+       return store;
     }
 
 }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SaveOptimizeTest.java

@@ -0,0 +1,580 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+
+package org.eclipse.jetty.server.session;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.util.StringUtil;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.StacklessLogging;
+import org.junit.Test;
+
+
+
+/**
+ * SaveOptimizeTest
+ *
+ * Test session save optimization.
+ */
+public class SaveOptimizeTest
+{
+
+    protected TestServlet _servlet;
+    protected TestServer _server1 = null;
+    
+  
+    
+    
+    /**
+     * Create and then invalidate a session in the same request.
+     * Use SessionCache.setSaveOnCreate(true) AND save optimization 
+     * and verify the session is actually saved.
+     * @throws Exception
+     */
+    @Test
+    public void testSessionCreateAndInvalidateWithSave() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 20;
+        int scavengePeriod = 3;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        storeFactory.setSavePeriodSec(10);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        }
+    }
+
+
+    /** 
+     * Test that repeated requests to a session where nothing changes does not do
+     * saves.
+     * @throws Exception
+     */
+    @Test
+    public void testCleanSessionWithinSavePeriod() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 600;
+        int scavengePeriod = 30;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        storeFactory.setSavePeriodSec(300);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                String sessionId = TestServer.extractSessionId(sessionCookie);
+
+
+                SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(data);
+                long firstSaved = data.getLastSaved();
+
+                //make a few requests to access the session but not change it
+                for (int i=0;i<5; i++)
+                {
+                    sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+
+                    // Perform a request to contextB with the same session cookie
+                    Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
+                    request.header("Cookie", sessionCookie);
+                    response = request.send();
+
+                    //check session is unchanged
+                    SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                    assertNotNull(d);
+                    assertEquals(firstSaved, d.getLastSaved());
+
+                    //slight pause between requests
+                    Thread.currentThread().sleep(500);
+                }
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        }
+    }
+    
+    
+    /**
+     * Test that a dirty session will always be saved regardless of
+     * save optimisation.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testDirtySession() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 600;
+        int scavengePeriod = 30;
+        int savePeriod = 5;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        storeFactory.setSavePeriodSec(savePeriod);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                String sessionId = TestServer.extractSessionId(sessionCookie);
+
+
+                SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(data);           
+                long lastSaved = data.getLastSaved();
+
+
+                // Perform a request to do nothing with the same session cookie
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+                Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session not saved
+                SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertEquals(lastSaved, d.getLastSaved());
+
+                // Perform a request to mutate the session
+                request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=mutate");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session is saved
+                d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertTrue(d.getLastSaved() > lastSaved);
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        }
+    }
+
+    /**
+     * Test that if the savePeriod is set, the session will only be saved
+     * after the savePeriod expires (if not dirty).
+     * @throws Exception
+     */
+    @Test
+    public void testCleanSessionAfterSavePeriod() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 600;
+        int scavengePeriod = 30;
+        int savePeriod = 5;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        storeFactory.setSavePeriodSec(savePeriod);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                String sessionId = TestServer.extractSessionId(sessionCookie);
+
+
+                SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(data);           
+                long lastSaved = data.getLastSaved();
+
+                //make another request, session should not change
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+
+                // Perform a request to do nothing with the same session cookie
+                Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session not saved
+                SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertEquals(lastSaved, d.getLastSaved());
+
+                //wait for the savePeriod to pass and then make another request, this should save the session
+                Thread.currentThread().sleep(1000*savePeriod);
+
+                // Perform a request to do nothing with the same session cookie
+                request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session is saved
+                d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertTrue(d.getLastSaved() > lastSaved);
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        }
+    }
+
+
+    /**
+     * Test that if we turn off caching of the session, then if a savePeriod
+     * is set, the session is still not saved unless the savePeriod expires.
+     * @throws Exception
+     */
+    @Test
+    public void testNoCacheWithSaveOptimization() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = -1;
+        int scavengePeriod = -1;
+        int savePeriod = 10;
+        //never cache sessions
+        NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory();
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        //optimize saves
+        storeFactory.setSavePeriodSec(savePeriod);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                String sessionId = TestServer.extractSessionId(sessionCookie);
+
+                SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(data);           
+                long lastSaved = data.getLastSaved();
+                assertTrue(lastSaved > 0); //check session created was saved
+
+
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+
+                // Perform a request to do nothing with the same session cookie, check the session object is different
+                Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop&check=diff");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session not saved
+                SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertEquals(lastSaved, d.getLastSaved()); 
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        } 
+    }
+
+    
+    /**
+     * Test changing the maxInactive on a session that is subject to save
+     * optimizations, and check that the session is saved, even if it is
+     * not otherwise dirty.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testChangeMaxInactiveWithSaveOptimisation () throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = -1;
+        int scavengePeriod = -1;
+        int savePeriod = 40;
+        //never cache sessions
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setSaveOnCreate(true);
+        TestSessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        //optimize saves
+        storeFactory.setSavePeriodSec(savePeriod);
+        _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        _servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(_servlet);
+        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        contextHandler.addServlet(holder, servletMapping);
+        _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        _server1.start();
+        int port1 = _server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            try
+            {
+                client.start();
+                String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
+
+                //make a request to set up a session on the server
+                ContentResponse response = client.GET(url);
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().get("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                String sessionId = TestServer.extractSessionId(sessionCookie);
+
+                SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(data);           
+                long lastSaved = data.getLastSaved();
+                assertTrue(lastSaved > 0); //check session created was saved
+
+
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+
+                // Perform a request to change maxInactive on session
+                Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=max&value=60");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                //check session is saved, even though the save optimisation interval has not passed
+                SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
+                assertNotNull(d);
+                assertTrue(d.getLastSaved() > lastSaved);
+                assertEquals(60000, d.getMaxInactiveMs());
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            _server1.stop();
+        } 
+        
+    }
+
+    public static class TestServlet extends HttpServlet
+    {
+        public String _id = null;
+        public SessionDataStore _store;
+        public HttpSession _firstSession = null;
+
+
+        public void setStore (SessionDataStore store)
+        {
+            _store = store;
+        }
+
+        @Override
+        protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException
+        {
+            String action = request.getParameter("action");
+
+            if (action != null && action.startsWith("create"))
+            {
+                HttpSession session = request.getSession(true);
+                _firstSession = session;
+                _id = session.getId();
+                session.setAttribute("value", new Integer(1));
+
+                String check = request.getParameter("check");
+                if (!StringUtil.isBlank(check) && _store != null)
+                {
+                    boolean exists;
+                    try
+                    {
+                        exists = _store.exists(_id);
+                    }
+                    catch (Exception e)
+                    {
+                        throw new ServletException (e);
+                    }
+
+                    if ("false".equalsIgnoreCase(check))   
+                        assertFalse(exists);
+                    else
+                        assertTrue(exists);
+                }
+            }
+            else if ("mutate".equalsIgnoreCase(action))
+            {
+                HttpSession session = request.getSession(false);
+                assertNotNull(session);
+                session.setAttribute("ttt", new Long(System.currentTimeMillis()));
+            }
+            else if ("max".equalsIgnoreCase(action))
+            {
+                int interval = Integer.parseInt(request.getParameter("value"));
+                HttpSession session = request.getSession(false);
+                assertNotNull(session);
+                session.setMaxInactiveInterval(interval);
+            }
+            else
+            {
+                //Don't change the session
+                HttpSession session = request.getSession(false);
+                assertNotNull(session);
+                
+                String check = request.getParameter("check");
+                if (!StringUtil.isBlank(check) && "same".equalsIgnoreCase(check))
+                {
+                    assertEquals(_firstSession, session);
+                }
+                else if (!StringUtil.isBlank(check) && "diff".equalsIgnoreCase(check))
+                {
+                    assertNotEquals(_firstSession, session);
+                }
+            }
+        }
+    }
+    
+}