jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1574520 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: delegate/SessionDelegate.java session/RefreshStrategy.java
Date Wed, 05 Mar 2014 15:04:24 GMT
Author: jukka
Date: Wed Mar  5 15:04:23 2014
New Revision: 1574520

URL: http://svn.apache.org/r1574520
Log:
OAK-1418: Read performance regression

Keep track of the last access time in the SessionDelegate instance
so we won't need to rely on side-effects in RefreshStrategy.needsRefresh()

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/RefreshStrategy.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java?rev=1574520&r1=1574519&r2=1574520&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java
Wed Mar  5 15:04:23 2014
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.oak.jcr.delegate;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static java.lang.System.nanoTime;
 import static org.apache.jackrabbit.api.stats.RepositoryStatistics.Type.SESSION_READ_COUNTER;
 import static org.apache.jackrabbit.api.stats.RepositoryStatistics.Type.SESSION_READ_DURATION;
 import static org.apache.jackrabbit.api.stats.RepositoryStatistics.Type.SESSION_WRITE_COUNTER;
@@ -68,6 +69,8 @@ public class SessionDelegate {
     private final IdentifierManager idManager;
     private final SessionStats sessionStats;
 
+    private volatile long lastAccessTimeNS = nanoTime();
+
     private final AtomicLong readCounter;
     private final AtomicLong readDuration;
     private final AtomicLong writeCounter;
@@ -112,6 +115,10 @@ public class SessionDelegate {
         return sessionStats;
     }
 
+    public long getNanosecondsSinceLastAccess() {
+        return nanoTime() - lastAccessTimeNS;
+    }
+
     public void refreshAtNextAccess() {
         refreshStrategy.refreshAtNextAccess();
     }
@@ -142,23 +149,30 @@ public class SessionDelegate {
     public synchronized <T> T perform(SessionOperation<T> sessionOperation)
             throws RepositoryException {
         // Synchronize to avoid conflicting refreshes from concurrent JCR API calls
+        long t0 = nanoTime();
         if (sessionOpCount == 0) {
-            // Refresh and precondition checks only for non re-entrant session operations
-            if (refreshStrategy.needsRefresh(sessionOperation)) {
+            // Refresh and precondition checks only for non re-entrant
+            // session operations. Don't refresh if this operation is a
+            // refresh operation itself or a save operation, which does an
+            // implicit refresh, or logout for obvious reasons.
+            if (!sessionOperation.isRefresh()
+                    && !sessionOperation.isSave()
+                    && !sessionOperation.isLogout()
+                    && refreshStrategy.needsRefresh(t0 - lastAccessTimeNS)) {
                 refresh(true);
                 refreshStrategy.refreshed();
                 updateCount++;
             }
             sessionOperation.checkPreconditions();
         }
-        long t0 = System.nanoTime();
         try {
             sessionOpCount++;
             T result =  sessionOperation.perform();
             logOperationDetails(sessionOperation);
             return result;
         } finally {
-            long dt = System.nanoTime() - t0;
+            lastAccessTimeNS = t0;
+            long dt = nanoTime() - t0;
             sessionOpCount--;
             if (sessionOperation.isUpdate()) {
                 sessionStats.write();

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/RefreshStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/RefreshStrategy.java?rev=1574520&r1=1574519&r2=1574520&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/RefreshStrategy.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/RefreshStrategy.java
Wed Mar  5 15:04:23 2014
@@ -19,20 +19,20 @@
 
 package org.apache.jackrabbit.oak.jcr.session;
 
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.NANOSECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
+import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
 import org.apache.jackrabbit.oak.jcr.session.operation.SessionOperation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Instances of this class determine whether a session needs to be refreshed base on
- * the current {@link SessionOperation operation} to be performed and past
- * {@link #refreshed() refreshes} and {@link #saved() saves}.
+ * Instances of this class determine whether a session needs to be refreshed
+ * before the next session operation is performed.
  * <p>
- * Before an operation is performed a session calls {@link #needsRefresh(SessionOperation)},
+ * Before an operation is performed a session calls {@link #needsRefresh(SessionDelegate)},
  * to determine whether the session needs to be refreshed first. To maintain a session strategy's
  * state sessions call {@link #refreshed()} right after each refresh operation and
  * {@link #saved()} right after each save operation.
@@ -60,28 +60,23 @@ public class RefreshStrategy {
     }
 
     /**
-     * Determine whether the session needs to refresh before {@code sessionOperation} is
performed.
+     * Determine whether the given session needs to refresh before the next
+     * session operation is performed.
      * <p>
-     * This implementation return {@code false} if either {@code sessionsOperation} is an
refresh
-     * operation or a save operation. Otherwise it returns {@code true} if and only if any
of the
-     * individual refresh strategies passed to the constructor returns {@code true}.
-     * @param sessionOperation  operation about to be performed
+     * This implementation returns {@code true} if and only if any of the
+     * individual refresh strategies passed to the constructor returns
+     * {@code true}.
+     *
+     * @param nanosecondsSinceLastAccess nanoseconds since last access
      * @return  {@code true} if and only if the session needs to refresh.
      */
-    public boolean needsRefresh(SessionOperation<?> sessionOperation) {
-        // Don't refresh if this operation is a refresh operation itself or
-        // a save operation, which does an implicit refresh
-        if (sessionOperation.isRefresh() || sessionOperation.isSave()
-                || sessionOperation.isLogout()) {
-            return false;
-        }
-
-        boolean refresh = false;
-        // Don't shortcut here since the individual strategies rely on side effects of this
call
+    public boolean needsRefresh(long nanosecondsSinceLastAccess) {
         for (RefreshStrategy r : refreshStrategies) {
-            refresh |= r.needsRefresh(sessionOperation);
+            if (r.needsRefresh(nanosecondsSinceLastAccess)) {
+                return true;
+            }
         }
-        return refresh;
+        return false;
     }
 
     /**
@@ -173,7 +168,7 @@ public class RefreshStrategy {
          * @return {@link #refresh}
          */
         @Override
-        public boolean needsRefresh(SessionOperation<?> sessionOperation) {
+        public boolean needsRefresh(long nanosecondsSinceLastAccess) {
             return refresh;
         }
 
@@ -231,44 +226,20 @@ public class RefreshStrategy {
      * This refresh strategy refreshes after a given timeout of inactivity.
      */
     public static class Timed extends RefreshStrategy {
+
         private final long interval;
-        private long lastAccessed = System.currentTimeMillis();
 
         /**
          * @param interval  Interval in seconds after which a session should refresh if there
was no
          *                  activity.
          */
         public Timed(long interval) {
-            this.interval = MILLISECONDS.convert(interval, SECONDS);
-        }
-
-        /**
-         * Called whenever {@code needsRefresh} determines that the time out interval was
exceeded.
-         * This default implementation always returns {@code true}. Descendants may override
this
-         * method to provide more refined behaviour.
-         * @param timeElapsed  the time that elapsed since the session was last accessed.
-         * @return {@code true}
-         */
-        protected boolean timeOut(long timeElapsed) {
-            return true;
+            this.interval = NANOSECONDS.convert(interval, SECONDS);
         }
 
         @Override
-        public boolean needsRefresh(SessionOperation<?> sessionOperation) {
-            long now = System.currentTimeMillis();
-            long timeElapsed = now - lastAccessed;
-            lastAccessed = now;
-            return timeElapsed > interval && timeOut(timeElapsed);
-        }
-
-        @Override
-        public void refreshed() {
-            lastAccessed = System.currentTimeMillis();
-        }
-
-        @Override
-        public void saved() {
-            lastAccessed = System.currentTimeMillis();
+        public boolean needsRefresh(long nanosecondsSinceLastAccess) {
+            return nanosecondsSinceLastAccess > interval;
         }
 
         @Override
@@ -278,14 +249,16 @@ public class RefreshStrategy {
     }
 
     /**
-     * This refresh strategy never refreshed the session but logs a warning if a session
has been
-     * idle for more than a given time.
+     * This refresh strategy never refreshed the session but logs
+     * a warning if a session has been idle for more than a given time.
      *
      * TODO replace logging with JMX monitoring. See OAK-941
      */
-    public static class LogOnce extends Timed {
+    public static class LogOnce extends RefreshStrategy {
         private final Exception initStackTrace = new Exception("The session was created here:");
 
+        private final long interval;
+
         private boolean warnIfIdle = true;
 
         /**
@@ -293,19 +266,20 @@ public class RefreshStrategy {
          *                  activity.
          */
         public LogOnce(long interval) {
-            super(interval);
+            this.interval = NANOSECONDS.convert(interval, SECONDS);
         }
 
         /**
          * Log once
-         * @param timeElapsed  the time that elapsed since the session was last accessed.
-         * @return  {@code false}
+         * @param nanosecondsSinceLastAccess nanoseconds since last access
+         * @return {@code false}
          */
         @Override
-        protected boolean timeOut(long timeElapsed) {
-            if (warnIfIdle) {
-                log.warn("This session has been idle for " + MINUTES.convert(
-                        timeElapsed, MILLISECONDS) + " minutes and might be out of date.
" +
+        public boolean needsRefresh(long nanosecondsSinceLastAccess) {
+            if (nanosecondsSinceLastAccess > interval && warnIfIdle) {
+                log.warn("This session has been idle for "
+                        + MINUTES.convert(nanosecondsSinceLastAccess, NANOSECONDS)
+                        + " minutes and might be out of date. " +
                         "Consider using a fresh session or explicitly refresh the session.",
                         initStackTrace);
                 warnIfIdle = false;
@@ -345,7 +319,7 @@ public class RefreshStrategy {
         }
 
         @Override
-        public boolean needsRefresh(SessionOperation<?> sessionOperation) {
+        public boolean needsRefresh(long nanosecondsSinceLastAccess) {
             // If the threadLocal counter differs from our seen sessionSaveCount so far then
             // some other session would have done a commit. If that is the case a refresh
would
             // be required



Mime
View raw message