jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1000950 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: SessionImpl.java WorkspaceManager.java observation/ObservationDispatcher.java session/SessionContext.java session/SessionState.java
Date Fri, 24 Sep 2010 16:45:02 GMT
Author: jukka
Date: Fri Sep 24 16:45:01 2010
New Revision: 1000950

URL: http://svn.apache.org/viewvc?rev=1000950&view=rev
Log:
JCR-2746: Sleep in possibly endless loop in ObservationDispatcher

Move the sleep to the end of SessionState.perform() after all the relevant locks have been
released

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/WorkspaceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ObservationDispatcher.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionContext.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionState.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java?rev=1000950&r1=1000949&r2=1000950&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
Fri Sep 24 16:45:01 2010
@@ -1238,14 +1238,4 @@ public class SessionImpl extends Abstrac
         }
     }
 
-    /**
-     * Delay the next operation, except if it is run using the given thread.
-     *
-     * @param exceptInThread the thread that shouldn't be delayed
-     * @param ms the delay in milliseconds
-     */
-    public void delayNextOperation(Thread exceptInThread, long ms) {
-        context.delayNextOperation(exceptInThread, ms);
-    }
-
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/WorkspaceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/WorkspaceManager.java?rev=1000950&r1=1000949&r2=1000950&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/WorkspaceManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/WorkspaceManager.java
Fri Sep 24 16:45:01 2010
@@ -16,11 +16,11 @@
  */
 package org.apache.jackrabbit.core;
 
-import javax.jcr.AccessDeniedException;
 import javax.jcr.NoSuchWorkspaceException;
 import javax.jcr.RepositoryException;
 import javax.security.auth.Subject;
 
+import org.apache.jackrabbit.core.observation.ObservationDispatcher;
 import org.apache.jackrabbit.core.state.SharedItemStateManager;
 import org.xml.sax.InputSource;
 
@@ -87,6 +87,12 @@ public class WorkspaceManager {
         return repository.getWorkspaceStateManager(workspaceName);
     }
 
+    // FIXME: This is a too low-level method. Refactor...
+    public ObservationDispatcher getObservationDispatcher(String workspaceName)
+            throws NoSuchWorkspaceException, RepositoryException {
+        return repository.getObservationDispatcher(workspaceName);
+    }
+
     // FIXME: There should be a better place for this. Refactor...
     public SessionImpl createSession(Subject subject, String workspaceName)
         throws RepositoryException {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ObservationDispatcher.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ObservationDispatcher.java?rev=1000950&r1=1000949&r2=1000950&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ObservationDispatcher.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/observation/ObservationDispatcher.java
Fri Sep 24 16:45:01 2010
@@ -218,8 +218,21 @@ public final class ObservationDispatcher
             }
         }
         eventQueue.add(new DispatchAction(events, getAsynchronousConsumers()));
-        int size = eventQueueSize.addAndGet(events.size());
-        if (size > MAX_QUEUED_EVENTS) {
+        eventQueueSize.addAndGet(events.size());
+    }
+
+    /**
+     * Checks if the observation event queue contains more than the
+     * configured {@link #MAX_QUEUED_EVENTS maximum number of events},
+     * and delays the current thread in such cases. No delay is added
+     * if the current thread is the observation thread, for example if
+     * an observation listener writes to the repository.
+     * <p>
+     * This method should only be called outside the scope of internal
+     * repository access locks.
+     */
+    public void delayIfEventQueueOverloaded() {
+        if (eventQueueSize.get() > MAX_QUEUED_EVENTS) {
             boolean logWarning = false;
             long now = System.currentTimeMillis();
             // log a warning at most every 5 seconds (to avoid filling the log file)
@@ -236,11 +249,10 @@ public final class ObservationDispatcher
                 if (logWarning) {
                     log.warn("Waiting");
                 }
-                if (eventQueueSize.get() > MAX_QUEUED_EVENTS) {
-                    // slow down the current session
-                    // but not here, because locks are held
-                    // (that may block an observation listener, which is not what we want)
-                    events.getSession().delayNextOperation(notificationThread, 100);
+                try {
+                    Thread.sleep(100);
+                } catch (InterruptedException e) {
+                    log.warn("Interrupted while rate-limiting writes", e);
                 }
             }
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionContext.java?rev=1000950&r1=1000949&r2=1000950&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionContext.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionContext.java
Fri Sep 24 16:45:01 2010
@@ -58,19 +58,6 @@ public class SessionContext implements N
     private static AtomicLong counter = new AtomicLong();
 
     /**
-     * The thread in which not to delay operations.
-     * If null, no operations are delayed.
-     * If not null, the next operation is delayed,
-     * except operations that are executed from this thread.
-     */
-    private Thread delayExceptInThread;
-
-    /**
-     * The delay in milliseconds.
-     */
-    private long delayMs;
-
-    /**
      * Creates a unique internal session name for a session with the
      * given user.
      *
@@ -374,33 +361,4 @@ public class SessionContext implements N
         return sessionName;
     }
 
-    /**
-     * Delay the next operation, except if it is run using the given thread.
-     *
-     * @param exceptInThread the thread that shouldn't be delayed
-     * @param ms the delay in milliseconds
-     */
-    public void delayNextOperation(Thread exceptInThread, long ms) {
-        this.delayExceptInThread = exceptInThread;
-        this.delayMs = ms;
-    }
-
-    /**
-     * Delay a session operation if it is necessary.
-     */
-    public void delayIfNecessary() {
-        if (delayExceptInThread != null) {
-            Thread currentThread = Thread.currentThread();
-            if (currentThread != delayExceptInThread) {
-                try {
-                    Thread.sleep(delayMs);
-                } catch (InterruptedException e) {
-                    // ignore
-                }
-                // don't delay the next operation
-                delayExceptInThread = null;
-            }
-        }
-    }
-
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionState.java?rev=1000950&r1=1000949&r2=1000950&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionState.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/session/SessionState.java
Fri Sep 24 16:45:01 2010
@@ -22,6 +22,8 @@ import java.util.concurrent.locks.Reentr
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 
+import org.apache.jackrabbit.core.WorkspaceManager;
+import org.apache.jackrabbit.core.observation.ObservationDispatcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -161,6 +163,7 @@ public class SessionState {
             lock.lock();
         }
 
+        boolean isOutermostWriteOperation = false;
         try {
             // Check that the session is still alive
             checkAlive();
@@ -171,7 +174,9 @@ public class SessionState {
             if (!wasWriteOperation
                     && operation instanceof SessionWriteOperation) {
                 isWriteOperation = true;
+                isOutermostWriteOperation = true;
             }
+
             try {
                 // Perform the actual operation, optionally with debug logs
                 if (log.isDebugEnabled()) {
@@ -190,7 +195,6 @@ public class SessionState {
                         }
                     }
                 } else {
-                    context.delayIfNecessary();
                     return operation.perform(context);
                 }
             } finally {
@@ -198,6 +202,18 @@ public class SessionState {
             }
         } finally {
             lock.unlock();
+
+            // Delay return from a write operation if the observation queue
+            // is being overloaded. This needs to be done after releasing
+            // the (outermost) write locks to prevent potential deadlocks.
+            // See https://issues.apache.org/jira/browse/JCR-2746
+            if (isOutermostWriteOperation) {
+                WorkspaceManager manager =
+                    context.getRepositoryContext().getWorkspaceManager();
+                ObservationDispatcher dispatcher =
+                    manager.getObservationDispatcher(context.getWorkspace().getName());
+                dispatcher.delayIfEventQueueOverloaded();
+            }
         }
     }
 



Mime
View raw message