Issue #2609 Updates and renaming. WIP.

Signed-off-by: Jan Bartel <janb@webtide.com>

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

@@ -615,9 +615,10 @@ public Set<String> doGetExpired(Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
-        //Get sessions managed by any node that expired before the given time limit
+        // Get sessions managed by any node that expired at or before the given
+        // time limit
         Set<String> expired = new HashSet<String>();
         try
         {    

jetty-hazelcast/src/main/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStore.java

@@ -163,7 +163,7 @@ public Set<String> doGetExpired( Set<String> candidates )
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
         // TODO
         return Collections.emptySet();

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

@@ -190,7 +190,7 @@ public Set<String> doGetExpired(Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
         // TODO
         return Collections.emptySet();
@@ -201,8 +201,7 @@ public Set<String> doGetOldExpired(long timeLimit)
     @Override
     public void cleanOrphans(long timeLimit)
     {
-        //Implemented by setting a timeout on the items that are put in infinispan. After
-        //the item has been inactive for the timeout, infinispan will delete it.
+        // TODO
     }
 
 

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

@@ -418,14 +418,15 @@ public Set<String> doGetExpired(Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
-        //now ask mongo to find sessions last managed by any nodes that expired a while ago 
+        // now ask mongo to find sessions for this context, last managed by any
+        // node, that expired before timeLimit
         Set<String> expiredSessions = new HashSet<>();
 
         BasicDBObject query = new BasicDBObject();
         BasicDBObject gt = new BasicDBObject(__EXPIRY, new BasicDBObject("$gt", 0));
-        BasicDBObject lt = new BasicDBObject (__EXPIRY, new BasicDBObject("$lt", timeLimit));
+        BasicDBObject lt = new BasicDBObject(__EXPIRY, new BasicDBObject("$le", timeLimit));
         BasicDBList list = new BasicDBList();
         list.add(gt);
         list.add(lt);

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

@@ -78,29 +78,30 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem
 
 
     /**
-     * Implemented by subclasses to find sessions for this context in the store that
-     * expired at or before the timeLimit and thus not being actively managed by
-     * any node. This method is only called periodically (the period
+     * Implemented by subclasses to find sessions for this context in the store
+     * that expired at or before the timeLimit and thus not being actively
+     * managed by any node. This method is only called periodically (the period
      * is configurable) to avoid putting too much load on the store.
      * 
-     * @param timeLimit the upper limit of expiry times to check. Sessions
-     * expired at or before this time will match.
+     * @param before the upper limit of expiry times to check. Sessions expired
+     *            at or before this timestamp will match.
      * 
-     * @return the empty set if there are no sessions expired as at the timeLimit, or
-     * otherwise a set of session ids.
+     * @return the empty set if there are no sessions expired as at the time, or
+     *         otherwise a set of session ids.
      */
-    public abstract Set<String> doGetOldExpired (long timeLimit);
+    public abstract Set<String> doGetExpired (long before);
 
 
     /**
      * Implemented by subclasses to delete sessions for other contexts that
-     * expired at or before the timeLimit. These are 'orphaned' sessions
-     * that are no longer being actively managed by any node. These are 
-     * explicitly sessions that do NOT belong to this context (other mechanisms
-     * such as doGetOrphanedExpired take care of those). As they don't belong
-     * to this context, they could not be loaded by us.
+     * expired at or before the timeLimit. These are 'orphaned' sessions that
+     * are no longer being actively managed by any node. These are explicitly
+     * sessions that do NOT belong to this context (other mechanisms such as
+     * doGetExpired take care of those). As they don't belong to this context,
+     * they could not be loaded by us.
      * 
-     * This is called only periodically to avoid placing excessive load on the store.
+     * This is called only periodically to avoid placing excessive load on the
+     * store.
      * 
      * @param timeLimit
      */
@@ -195,54 +196,55 @@ public void run ()
     @Override
     public Set<String> getExpired(Set<String> candidates)
     {
-        //always verify the set of candidates we've been given
+        // 1. always verify the set of candidates we've been given
         //by the sessioncache
         Set<String> expired = doGetExpired (candidates);
 
         long now = System.currentTimeMillis();
 
-        //only periodically check the backing store to find other sessions
-        //in this context that expired long ago (ie not being actively managed
+        // 2. check the backing store to find other sessions
+        // in THIS context that expired long ago (ie cannot be actively managed
         //by any node)
-        long expiryTimeLimit = 0;
-        //if first check then find sessions that expired 3 grace periods ago.
-        //this ensures that on startup we don't find sessions that are expired
-        //but being managed by another node.
-        if (_lastExpiryCheckTime <= 0)
-            expiryTimeLimit = now - TimeUnit.SECONDS.toMillis(_gracePeriodSec*3);
-        else
+        try
         {
-            //only do the check once every gracePeriod to avoid expensive searches
-            if (now > (_lastExpiryCheckTime+TimeUnit.SECONDS.toMillis(_gracePeriodSec)))
-                expiryTimeLimit = now - TimeUnit.SECONDS.toMillis(_gracePeriodSec);
-        }
-        if (expiryTimeLimit > 0)
-        {   
-            if (LOG.isDebugEnabled()) LOG.debug("Searching for old expired sessions for context {}", _context.getCanonicalContextPath());
-            try
-            {
-                expired.addAll(doGetOldExpired(expiryTimeLimit));
-            }
-            finally
+            long expiryTimeLimit = 0;
+
+            // if we have never checked for old expired sessions, then only find
+            // those that are very old
+            if (_lastExpiryCheckTime <= 0)
+                expiryTimeLimit = now - TimeUnit.SECONDS.toMillis(_gracePeriodSec * 3);
+            else
             {
-                _lastExpiryCheckTime = now;
+                // only do the check once every gracePeriod to avoid expensive
+                // searches
+                if (now > (_lastExpiryCheckTime + TimeUnit.SECONDS.toMillis(_gracePeriodSec)))
+                    expiryTimeLimit = now - TimeUnit.SECONDS.toMillis(_gracePeriodSec);
             }
+            if (LOG.isDebugEnabled()) LOG.debug("Searching for sessions expired before {} for context {}", expiryTimeLimit, _context.getCanonicalContextPath());
+            expired.addAll(doGetExpired(expiryTimeLimit));
         }
-        //periodically comb the backing store to delete sessions for other
-        //other contexts that expired a long time ago (ie not being actively
+        finally
+        {
+            _lastExpiryCheckTime = now;
+        }
+
+
+        // 3. Periodically comb the backing store to delete sessions for OTHER
+        // contexts that expired a long time ago (ie not being actively
         //managed by any node).
-        if (_lastOrphanSweepTime > 0 && (now > (_lastOrphanSweepTime+TimeUnit.SECONDS.toMillis(10*_gracePeriodSec))))
-        {   
-            try
+        try
+        {
+            if (now > (_lastOrphanSweepTime + TimeUnit.SECONDS.toMillis(10 * _gracePeriodSec)))
             {
                 if (LOG.isDebugEnabled()) LOG.debug("Cleaning orphans");
                 cleanOrphans(now - TimeUnit.SECONDS.toMillis(10*_gracePeriodSec));
             }
-            finally
-            {
-                _lastOrphanSweepTime = now;            
-            }
         }
+        finally
+        {
+            _lastOrphanSweepTime = now;
+        }
+
         return expired;
     }
 

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

@@ -191,18 +191,18 @@ public Set<String> doGetExpired(final Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
-        final long now = System.currentTimeMillis();
         HashSet<String> expired = new HashSet<>();
 
-        //iterate over the files and work out which have expired
+        // iterate over the files and work out which expired at or
+        // before the time limit
         for (String filename:_sessionFileMap.values())
         {
             try
             {
                 long expiry = getExpiryFromFilename(filename);
-                if (expiry > 0 && expiry < timeLimit)
+                if (expiry > 0 && expiry <= timeLimit)
                     expired.add(getIdFromFilename(filename));
             }
             catch (Exception e)
@@ -228,13 +228,12 @@ public void sweepDisk()
 
 
     /**
-     * Check all session files that do not belong to this context and
-     * remove any that expired long ago (ie at least 5 gracePeriods ago).
+     * Check all session files that do not belong to this context and remove any
+     * that expired at or before the time limit.
      */
     public void sweepDisk(long timeLimit)
     {
-        //iterate over the files in the store dir and check expiry times
-
+        // iterate over the files in the store dir and check expiry times
         if (LOG.isDebugEnabled()) LOG.debug("Sweeping {} for old session files", _storeDir);
         try
         {

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

@@ -860,14 +860,15 @@ public Set<String> doGetExpired(Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
         Set<String> expired = new HashSet<>();
         //Get sessions for my context but managed by any node that expired at or before the timeLimit   
         try (Connection connection = _dbAdaptor.getConnection())
         {
             connection.setAutoCommit(true);
-            try (PreparedStatement selectExpiredSessions = _sessionTableSchema.getExpiredSessionsStatement(connection, _context.getCanonicalContextPath(), _context.getVhost(), timeLimit ))
+            try (PreparedStatement selectExpiredSessions = _sessionTableSchema.getExpiredSessionsStatement(connection, _context.getCanonicalContextPath(),
+                                                                                                           _context.getVhost(), timeLimit))
             {
                 if (LOG.isDebugEnabled()) 
                     LOG.debug("{}- Searching for sessions for context {} expired before {}",_context.getWorkerName(),_context.getCanonicalContextPath(), timeLimit);

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

@@ -72,7 +72,7 @@ public Set<String> doGetExpired(Set<String> candidates)
 
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
         return Collections.emptySet();
     }
@@ -102,6 +102,4 @@ public void cleanOrphans(long timeLimit)
         //noop
     }
 
-
-
 }

tests/test-sessions/test-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/sessions/MemcachedTestHelper.java

@@ -122,7 +122,7 @@ else if (sd.isExpiredAt(now))
 
 
         @Override
-        public Set<String> doGetOldExpired(long timeLimit)
+        public Set<String> doGetExpired(long timeLimit)
         {
             return Collections.emptySet();
         }

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

@@ -105,7 +105,7 @@ public Set<String> doGetExpired(Set<String> candidates)
     }
 
     @Override
-    public Set<String> doGetOldExpired(long timeLimit)
+    public Set<String> doGetExpired(long timeLimit)
     {
         Set<String> set =  new HashSet<>();
 

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

@@ -115,7 +115,7 @@ public Set<String> doGetExpired(Set<String> candidates)
         }
 
         @Override
-        public Set<String> doGetOldExpired(long timeLimit)
+        public Set<String> doGetExpired(long timeLimit)
         {
             return Collections.emptySet();
         }

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

@@ -111,7 +111,7 @@ public Set<String> doGetExpired(Set<String> candidates)
         }
 
         @Override
-        public Set<String> doGetOldExpired(long timeLimit)
+        public Set<String> doGetExpired(long timeLimit)
         {
             return Collections.emptySet();
         }