You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by dp...@apache.org on 2006/12/21 15:42:29 UTC

svn commit: r489376 - in /jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster: ClusterNode.java FileJournal.java FileRevision.java Journal.java RecordProcessor.java

Author: dpfister
Date: Thu Dec 21 06:42:28 2006
New Revision: 489376

URL: http://svn.apache.org/viewvc?view=rev&rev=489376
Log:
JCR-623 - Clustering
+ Release write mutex when error occurs
+ Catch unexpected errors in synchronization listener
+ Add close() method to journal

Modified:
    jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/ClusterNode.java
    jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileJournal.java
    jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileRevision.java
    jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/Journal.java
    jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/RecordProcessor.java

Modified: jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/ClusterNode.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/ClusterNode.java?view=diff&rev=489376&r1=489375&r2=489376
==============================================================================
--- jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/ClusterNode.java (original)
+++ jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/ClusterNode.java Thu Dec 21 06:42:28 2006
@@ -25,14 +25,9 @@
 import org.apache.jackrabbit.core.observation.EventState;
 import org.apache.jackrabbit.core.observation.EventStateCollection;
 import org.apache.jackrabbit.core.state.ChangeLog;
-import org.apache.jackrabbit.name.Path;
-import org.apache.jackrabbit.name.QName;
 import EDU.oswego.cs.dl.util.concurrent.Mutex;
 
-import javax.jcr.observation.Event;
-import javax.jcr.Session;
 import javax.jcr.RepositoryException;
-import java.util.Set;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -195,6 +190,13 @@
             } catch (ClusterException e) {
                 String msg = "Periodic sync of journal failed: " + e.getMessage();
                 log.error(msg);
+            } catch (Exception e) {
+                String msg = "Unexpected error while syncing of journal: " + e.getMessage();
+                log.error(msg, e);
+            } catch (Error e) {
+                String msg = "Unexpected error while syncing of journal: " + e.getMessage();
+                log.error(msg, e);
+                throw e;
             }
         }
     }
@@ -219,12 +221,15 @@
     }
 
     /**
-     * {@inheritDoc}
+     * Stops this cluster node.
      */
     public synchronized void stop() {
-        status = STOPPED;
+        if (status == STARTED) {
+            status = STOPPED;
 
-        notifyAll();
+            journal.close();
+            notifyAll();
+        }
     }
 
     /**
@@ -644,11 +649,6 @@
         private List events;
 
         /**
-         * Last used session for event sources.
-         */
-        private Session lastSession;
-
-        /**
          * {@inheritDoc}
          */
         public void start(String workspace) {
@@ -668,36 +668,7 @@
         /**
          * {@inheritDoc}
          */
-        public void process(int type, NodeId parentId, Path parentPath, NodeId childId,
-                            Path.PathElement childRelPath, QName ntName, Set mixins, String userId) {
-
-            EventState event = null;
-
-            switch (type) {
-                case Event.NODE_ADDED:
-                    event = EventState.childNodeAdded(parentId, parentPath, childId, childRelPath,
-                            ntName, mixins, getOrCreateSession(userId));
-                    break;
-                case Event.NODE_REMOVED:
-                    event = EventState.childNodeRemoved(parentId, parentPath, childId, childRelPath,
-                            ntName, mixins, getOrCreateSession(userId));
-                    break;
-                case Event.PROPERTY_ADDED:
-                    event = EventState.propertyAdded(parentId, parentPath, childRelPath,
-                            ntName, mixins, getOrCreateSession(userId));
-                    break;
-                case Event.PROPERTY_CHANGED:
-                    event = EventState.propertyChanged(parentId, parentPath, childRelPath,
-                            ntName, mixins, getOrCreateSession(userId));
-                    break;
-                case Event.PROPERTY_REMOVED:
-                    event = EventState.propertyRemoved(parentId, parentPath, childRelPath,
-                            ntName, mixins, getOrCreateSession(userId));
-                    break;
-                default:
-                    String msg = "Unexpected event type: " + type;
-                    log.warn(msg);
-            }
+        public void process(EventState event) {
             events.add(event);
         }
 
@@ -831,19 +802,6 @@
                 String msg = "Unable to deliver update events: " + e.getMessage();
                 log.error(msg);
             }
-        }
-
-        /**
-         * Return a session matching a certain user id.
-         *
-         * @param userId user id
-         * @return session
-         */
-        private Session getOrCreateSession(String userId) {
-            if (lastSession == null || !lastSession.getUserID().equals(userId)) {
-                lastSession = new ClusterSession(userId);
-            }
-            return lastSession;
         }
     }
 }

Modified: jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileJournal.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileJournal.java?view=diff&rev=489376&r1=489375&r2=489376
==============================================================================
--- jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileJournal.java (original)
+++ jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileJournal.java Thu Dec 21 06:42:28 2006
@@ -45,6 +45,9 @@
 
 import EDU.oswego.cs.dl.util.concurrent.Mutex;
 
+import javax.jcr.observation.Event;
+import javax.jcr.Session;
+
 /**
  * File-based journal implementation. A directory specified as <code>directory</code>
  * bean property will contain log files and a global revision file, containing the
@@ -169,6 +172,11 @@
     private FileRecord record;
 
     /**
+     * Last used session for event sources.
+     */
+    private Session lastSession;
+
+    /**
      * Bean getter for journal directory.
      * @return directory
      */
@@ -189,7 +197,7 @@
      * @return revision file
      */
     public String getRevision() {
-        return directory;
+        return revision;
     }
 
     /**
@@ -302,7 +310,7 @@
                 }
             } catch (IOException e) {
                 String msg = "Unable to iterate over modified records: " + e.getMessage();
-                throw new JournalException(msg);
+                throw new JournalException(msg, e);
 
             } finally {
                 try {
@@ -359,8 +367,8 @@
                         mixins.add(in.readQName());
                     }
                     String userId = in.readString();
-                    processor.process(type, parentId, parentPath, childId,
-                            childRelPath, ntName, mixins, userId);
+                    processor.process(createEventState(type, parentId, parentPath, childId,
+                            childRelPath, ntName, mixins, userId));
                 } else if (c == 'L') {
                     NodeId nodeId = in.readNodeId();
                     boolean isLock = in.readBoolean();
@@ -639,6 +647,7 @@
         } finally {
             if (!prepared) {
                 globalRevision.unlock();
+                writeMutex.release();
             }
         }
     }
@@ -693,6 +702,62 @@
                 writeMutex.release();
             }
         }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public void close() {}
+
+    /**
+     * Create an event state.
+     *
+     * @param type event type
+     * @param parentId parent id
+     * @param parentPath parent path
+     * @param childId child id
+     * @param childRelPath child relative path
+     * @param ntName ndoe type name
+     * @param userId user id
+     * @return event
+     */
+    protected EventState createEventState(int type, NodeId parentId, Path parentPath,
+                                          NodeId childId, Path.PathElement childRelPath,
+                                          QName ntName, Set mixins, String userId) {
+        switch (type) {
+            case Event.NODE_ADDED:
+                return EventState.childNodeAdded(parentId, parentPath, childId, childRelPath,
+                        ntName, mixins, getOrCreateSession(userId));
+            case Event.NODE_REMOVED:
+                return EventState.childNodeRemoved(parentId, parentPath, childId, childRelPath,
+                        ntName, mixins, getOrCreateSession(userId));
+            case Event.PROPERTY_ADDED:
+                return EventState.propertyAdded(parentId, parentPath, childRelPath,
+                        ntName, mixins, getOrCreateSession(userId));
+            case Event.PROPERTY_CHANGED:
+                return EventState.propertyChanged(parentId, parentPath, childRelPath,
+                        ntName, mixins, getOrCreateSession(userId));
+            case Event.PROPERTY_REMOVED:
+                return EventState.propertyRemoved(parentId, parentPath, childRelPath,
+                        ntName, mixins, getOrCreateSession(userId));
+            default:
+                String msg = "Unexpected event type: " + type;
+                throw new IllegalArgumentException(msg);
+        }
+    }
+
+
+    /**
+     * Return a session matching a certain user id.
+     *
+     * @param userId user id
+     * @return session
+     */
+    protected Session getOrCreateSession(String userId) {
+        if (lastSession == null || !lastSession.getUserID().equals(userId)) {
+            lastSession = new ClusterSession(userId);
+        }
+        return lastSession;
     }
 
     /**

Modified: jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileRevision.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileRevision.java?view=diff&rev=489376&r1=489375&r2=489376
==============================================================================
--- jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileRevision.java (original)
+++ jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/FileRevision.java Thu Dec 21 06:42:28 2006
@@ -93,6 +93,7 @@
                         String msg = "I/O error while closing file " + file.getPath() + ": " + e.getMessage();
                         log.warn(msg);
                     }
+                    raf = null;
                 }
             }
         }
@@ -109,17 +110,16 @@
             } catch (IOException e) {
                 String msg = "I/O error while releasing lock: " + e.getMessage();
                 log.warn(msg);
-            } finally {
-                lock = null;
             }
+            lock = null;
+
             try {
                 raf.close();
             } catch (IOException e) {
                 String msg = "I/O error while closing file: " + e.getMessage();
                 log.warn(msg);
-            } finally {
-                raf = null;
             }
+            raf = null;
         }
     }
 
@@ -130,9 +130,9 @@
      * @throws JournalException if some error occurs
      */
     public long get() throws JournalException {
-        try {
-            lock(true);
+        lock(true);
 
+        try {
             long value = 0;
             if (raf.length() > 0) {
                 raf.seek(0L);
@@ -154,9 +154,9 @@
      * @throws JournalException if some error occurs
      */
     public void set(long value) throws JournalException {
-        try {
-            lock(false);
+        lock(false);
 
+        try {
             raf.seek(0L);
             raf.writeLong(value);
         } catch (IOException e) {

Modified: jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/Journal.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/Journal.java?view=diff&rev=489376&r1=489375&r2=489376
==============================================================================
--- jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/Journal.java (original)
+++ jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/Journal.java Thu Dec 21 06:42:28 2006
@@ -117,4 +117,9 @@
      * End this update operation and discards changes made to the journal.
      */
     public void cancel();
+
+    /**
+     * Close this journal. This should release any resources still held by this journal.
+     */
+    public void close();
 }

Modified: jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/RecordProcessor.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/RecordProcessor.java?view=diff&rev=489376&r1=489375&r2=489376
==============================================================================
--- jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/RecordProcessor.java (original)
+++ jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster/RecordProcessor.java Thu Dec 21 06:42:28 2006
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.core.cluster;
 
 import org.apache.jackrabbit.core.NodeId;
+import org.apache.jackrabbit.core.observation.EventState;
 import org.apache.jackrabbit.name.Path;
 import org.apache.jackrabbit.name.QName;
 
@@ -45,16 +46,9 @@
     /**
      * Process an event.
      *
-     * @param type event type
-     * @param parentId parent id
-     * @param parentPath parent path
-     * @param childId child id
-     * @param childRelPath child relative path
-     * @param ntName ndoe type name
-     * @param userId user id
+     * @param event event
      */
-    public void process(int type, NodeId parentId, Path parentPath, NodeId childId,
-                        Path.PathElement childRelPath, QName ntName, Set mixins, String userId);
+    public void process(EventState event);
 
     /**
      * Process a lock operation.



Re: svn commit: r489376 - in /jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster: ClusterNode.java FileJournal.java FileRevision.java Journal.java RecordProcessor.java

Posted by Dominique Pfister <do...@day.com>.
Hi Jukka,

> Please do not commit stuff to the 1.2 branch unless there's a specific
> *new* issue that I've OK'd for the release. I'm trying to prepare the
> first release candidate and need to be aware of changes in the branch.

Sorry about that. I recently found some flaws in my clustering
implementation, that IMHO challenged the closing of JCR-623.
Yesterday, I noticed that the fixes for those errors didn't make it
into the soon to be released 1.2 branch and I panicked.

> More generally I'd suggest to open new issues and tag commits
> accordingly instead of reusing JCR-623 for additional fixes. Each of
> the three mentioned changes would IMHO warrant individual issues; two
> bug reports and one improvent request. Doing that would make the
> clustering work much more transparent as the rationale for the changes
> would be more explicit and the complete list of open and closed issues
> would be readily available.

I completely agree. Once again, sorry for the trouble this may have caused.

Cheers
Dominique

Re: svn commit: r489376 - in /jackrabbit/branches/1.2/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cluster: ClusterNode.java FileJournal.java FileRevision.java Journal.java RecordProcessor.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On 12/21/06, dpfister@apache.org <dp...@apache.org> wrote:
> Author: dpfister
> Date: Thu Dec 21 06:42:28 2006
> New Revision: 489376
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=489376
> Log:
> JCR-623 - Clustering
> + Release write mutex when error occurs
> + Catch unexpected errors in synchronization listener
> + Add close() method to journal
>
> Modified:
>     jackrabbit/branches/1.2/[...]

Please do not commit stuff to the 1.2 branch unless there's a specific
*new* issue that I've OK'd for the release. I'm trying to prepare the
first release candidate and need to be aware of changes in the branch.

More generally I'd suggest to open new issues and tag commits
accordingly instead of reusing JCR-623 for additional fixes. Each of
the three mentioned changes would IMHO warrant individual issues; two
bug reports and one improvent request. Doing that would make the
clustering work much more transparent as the rationale for the changes
would be more explicit and the complete list of open and closed issues
would be readily available.

I'm going to let this one go as the changes only affect the new
cluster classes, but please avoid similar commits in future.

BR,

Jukka Zitting