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
