You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by th...@apache.org on 2012/07/31 09:07:19 UTC
svn commit: r1367435 - in /jackrabbit/branches/2.4/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/
main/java/org/apache/jackrabbit/core/data/
test/java/org/apache/jackrabbit/core/data/
Author: thomasm
Date: Tue Jul 31 07:07:18 2012
New Revision: 1367435
URL: http://svn.apache.org/viewvc?rev=1367435&view=rev
Log:
JCR-3369 Garbage collector improvements
Added:
jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java
Modified:
jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java
jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java
jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java?rev=1367435&r1=1367434&r2=1367435&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java Tue Jul 31 07:07:18 2012
@@ -97,6 +97,16 @@ public class CachingHierarchyManager ext
private boolean consistencyCheckEnabled;
/**
+ * Log interval for item state exceptions.
+ */
+ private static final int ITEM_STATE_EXCEPTION_LOG_INTERVAL_MILLIS = 60 * 1000;
+
+ /**
+ * Last time-stamp item state exception was logged with a stacktrace.
+ */
+ private long itemStateExceptionLogTimestamp = 0;
+
+ /**
* Create a new instance of this class.
*
* @param rootNodeId root node id
@@ -151,8 +161,9 @@ public class CachingHierarchyManager ext
try {
return resolvePath(elements, element.getDepth() + 1, entry.getId(), typesAllowed);
} catch (ItemStateException e) {
- String msg = "failed to retrieve state of intermediary node";
- log.debug(msg);
+ String msg = "failed to retrieve state of intermediary node for entry: "
+ + entry.getId() + ", path: " + path.getString();
+ logItemStateException(msg, e);
throw new RepositoryException(msg, e);
}
}
@@ -833,6 +844,22 @@ public class CachingHierarchyManager ext
}
/**
+ * Helper method to log item state exception with stack trace every so often.
+ *
+ * @param logMessage log message
+ * @param e item state exception
+ */
+ private void logItemStateException(String logMessage, ItemStateException e) {
+ long now = System.currentTimeMillis();
+ if ((now - itemStateExceptionLogTimestamp) >= ITEM_STATE_EXCEPTION_LOG_INTERVAL_MILLIS) {
+ itemStateExceptionLogTimestamp = now;
+ log.debug(logMessage, e);
+ } else {
+ log.debug(logMessage);
+ }
+ }
+
+ /**
* Entry in the LRU list
*/
private class LRUEntry {
Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?rev=1367435&r1=1367434&r2=1367435&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java Tue Jul 31 07:07:18 2012
@@ -1391,7 +1391,7 @@ public class RepositoryImpl extends Abst
PersistenceManager pm = vm.getPersistenceManager();
pmList.add(pm);
String[] wspNames = getWorkspaceNames();
- Session[] sessions = new Session[wspNames.length];
+ SessionImpl[] sessions = new SessionImpl[wspNames.length];
for (int i = 0; i < wspNames.length; i++) {
String wspName = wspNames[i];
WorkspaceInfo wspInfo = getWorkspaceInfo(wspName);
Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java?rev=1367435&r1=1367434&r2=1367435&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java Tue Jul 31 07:07:18 2012
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.core.data;
import org.apache.jackrabbit.api.management.DataStoreGarbageCollector;
import org.apache.jackrabbit.api.management.MarkEventListener;
+import org.apache.jackrabbit.core.SessionImpl;
import org.apache.jackrabbit.core.id.NodeId;
import org.apache.jackrabbit.core.id.PropertyId;
import org.apache.jackrabbit.core.observation.SynchronousEventListener;
@@ -76,7 +77,7 @@ import javax.jcr.observation.Observation
public class GarbageCollector implements DataStoreGarbageCollector {
/** logger instance */
- private static final Logger LOG = LoggerFactory.getLogger(GarbageCollector.class);
+ static final Logger LOG = LoggerFactory.getLogger(GarbageCollector.class);
private MarkEventListener callback;
@@ -92,12 +93,14 @@ public class GarbageCollector implements
private final IterablePersistenceManager[] pmList;
- private final Session[] sessionList;
+ private final SessionImpl[] sessionList;
private final AtomicBoolean closed = new AtomicBoolean();
private boolean persistenceManagerScan;
+ private volatile RepositoryException observationException;
+
/**
* Create a new garbage collector.
* This method is usually not called by the application, it is called
@@ -109,7 +112,7 @@ public class GarbageCollector implements
*/
public GarbageCollector(
DataStore dataStore, IterablePersistenceManager[] list,
- Session[] sessionList) {
+ SessionImpl[] sessionList) {
this.store = dataStore;
this.pmList = list;
this.persistenceManagerScan = list != null;
@@ -162,7 +165,7 @@ public class GarbageCollector implements
}
if (pmList == null || !persistenceManagerScan) {
- for (Session s : sessionList) {
+ for (SessionImpl s : sessionList) {
scanNodes(s);
}
} else {
@@ -174,11 +177,11 @@ public class GarbageCollector implements
}
}
- private void scanNodes(Session session) throws RepositoryException {
+ private void scanNodes(SessionImpl session) throws RepositoryException {
- // add a listener to get 'new' nodes
- // actually, new nodes are not the problem, but moved nodes
- listeners.add(new Listener(session));
+ // add a listener to get 'moved' nodes
+ Session clonedSession = session.createSession(session.getWorkspace().getName());
+ listeners.add(new Listener(this, clonedSession));
// adding a link to a BLOB updates the modified date
// reading usually doesn't, but when scanning, it does
@@ -234,14 +237,11 @@ public class GarbageCollector implements
public void stopScan() throws RepositoryException {
if (listeners.size() > 0) {
for (Listener listener : listeners) {
- try {
- listener.stop();
- } catch (Exception e) {
- throw new RepositoryException(e);
- }
+ listener.stop();
}
listeners.clear();
}
+ checkObservationException();
}
/**
@@ -309,6 +309,7 @@ public class GarbageCollector implements
} catch (InvalidItemStateException e) {
LOG.debug("Node removed concurrently - ignoring", e);
}
+ checkObservationException();
}
private void rememberNode(String path) {
@@ -368,6 +369,24 @@ public class GarbageCollector implements
}
}
+ private void checkObservationException() throws RepositoryException {
+ RepositoryException e = observationException;
+ if (e != null) {
+ observationException = null;
+ String message = "Exception while processing concurrent events";
+ LOG.warn(message, e);
+ e = new RepositoryException(message, e);
+ }
+ }
+
+ void onObservationException(Exception e) {
+ if (e instanceof RepositoryException) {
+ observationException = (RepositoryException) e;
+ } else {
+ observationException = new RepositoryException(e);
+ }
+ }
+
/**
* Auto-close in case the application didn't call it explicitly.
*/
@@ -382,27 +401,24 @@ public class GarbageCollector implements
*/
class Listener implements SynchronousEventListener {
+ private final GarbageCollector gc;
private final Session session;
-
private final ObservationManager manager;
- private Exception lastException;
-
- Listener(Session session)
+ Listener(GarbageCollector gc, Session session)
throws UnsupportedRepositoryOperationException,
RepositoryException {
+ this.gc = gc;
this.session = session;
Workspace ws = session.getWorkspace();
manager = ws.getObservationManager();
- manager.addEventListener(this, Event.NODE_ADDED, "/", true, null,
+ manager.addEventListener(this, Event.NODE_MOVED, "/", true, null,
null, false);
}
- void stop() throws Exception {
- if (lastException != null) {
- throw lastException;
- }
+ void stop() throws RepositoryException {
manager.removeEventListener(this);
+ session.logout();
}
public void onEvent(EventIterator events) {
@@ -427,7 +443,12 @@ public class GarbageCollector implements
// ignore
}
} catch (Exception e) {
- lastException = e;
+ gc.onObservationException(e);
+ try {
+ stop();
+ } catch (RepositoryException e2) {
+ LOG.warn("Exception removing the observation listener - ignored", e2);
+ }
}
}
}
Added: jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java?rev=1367435&view=auto
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java (added)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java Tue Jul 31 07:07:18 2012
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.data;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.SimpleCredentials;
+import javax.jcr.ValueFactory;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.jackrabbit.api.JackrabbitRepository;
+import org.apache.jackrabbit.api.JackrabbitRepositoryFactory;
+import org.apache.jackrabbit.api.management.MarkEventListener;
+import org.apache.jackrabbit.core.RepositoryFactoryImpl;
+import org.apache.jackrabbit.core.SessionImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Test case for the scenario where the GC thread traverses the workspace and at
+ * some point, a subtree that the GC thread did not see yet is moved to a location
+ * that the thread has already traversed. The GC thread should not ignore binaries
+ * references by this subtree and eventually delete them.
+ */
+public class GCSubtreeMoveTest extends TestCase {
+
+ private static final Logger logger = LoggerFactory.getLogger(GCSubtreeMoveTest.class);
+
+ private String testDirectory;
+ private JackrabbitRepository repository;
+ private Session sessionGarbageCollector;
+ private Session sessionMover;
+
+ public void setUp() throws IOException {
+ testDirectory = "target/" + getClass().getSimpleName() + "/" + getName();
+ FileUtils.deleteDirectory(new File(testDirectory));
+ }
+
+ public void tearDown() throws IOException {
+ sessionGarbageCollector.logout();
+ sessionMover.logout();
+ repository.shutdown();
+
+ repository = null;
+ sessionGarbageCollector = null;
+ sessionMover = null;
+
+ FileUtils.deleteDirectory(new File(testDirectory));
+ testDirectory = null;
+ }
+
+ public void test() {
+ setupRepository();
+
+ GarbageCollector garbageCollector = setupGarbageCollector();
+ // To make sure even listener for NODE_ADDED is registered in GC.
+ garbageCollector.setPersistenceManagerScan(false);
+
+ assertEquals(0, getBinaryCount(garbageCollector));
+ setupNodes();
+ assertEquals(1, getBinaryCount(garbageCollector));
+ garbageCollector.getDataStore().clearInUse();
+
+ garbageCollector.setMarkEventListener(new MarkEventListener() {
+
+ public void beforeScanning(Node node) throws RepositoryException {
+ String path = node.getPath();
+ if (path.startsWith("/node")) {
+ log("Traversing: " + node.getPath());
+ }
+
+ if ("/node1".equals(node.getPath())) {
+ String from = "/node2/node3";
+ String to = "/node0/node3";
+ log("Moving " + from + " -> " + to);
+ sessionMover.move(from, to);
+ sessionMover.save();
+ sleepForFile();
+ }
+ }
+ });
+
+ try {
+ garbageCollector.getDataStore().clearInUse();
+ garbageCollector.mark();
+ garbageCollector.stopScan();
+ sleepForFile();
+ int numberOfDeleted = garbageCollector.sweep();
+ log("Number of deleted: " + numberOfDeleted);
+ // Binary data should still be there.
+ assertEquals(1, getBinaryCount(garbageCollector));
+ } catch (RepositoryException e) {
+ e.printStackTrace();
+ failWithException(e);
+ } finally {
+ garbageCollector.close();
+ }
+ }
+
+ private void setupNodes() {
+ try {
+ Node rootNode = sessionMover.getRootNode();
+ rootNode.addNode("node0");
+ rootNode.addNode("node1");
+ Node node2 = rootNode.addNode("node2");
+ Node node3 = node2.addNode("node3");
+ Node nodeWithBinary = node3.addNode("node-with-binary");
+ ValueFactory vf = sessionGarbageCollector.getValueFactory();
+ nodeWithBinary.setProperty("prop", vf.createBinary(new RandomInputStream(10, 1000)));
+ sessionMover.save();
+ sleepForFile();
+ } catch (RepositoryException e) {
+ failWithException(e);
+ }
+ }
+
+ private void sleepForFile() {
+ // Make sure the file is old (access time resolution is 2 seconds)
+ try {
+ Thread.sleep(2200);
+ } catch (InterruptedException ignore) {
+ }
+ }
+
+ private void setupRepository() {
+ JackrabbitRepositoryFactory repositoryFactory = new RepositoryFactoryImpl();
+ createRepository(repositoryFactory);
+ login();
+ }
+
+ private void createRepository(JackrabbitRepositoryFactory repositoryFactory) {
+ Properties prop = new Properties();
+ prop.setProperty("org.apache.jackrabbit.repository.home", testDirectory);
+ prop.setProperty("org.apache.jackrabbit.repository.conf", testDirectory + "/repository.xml");
+ try {
+ repository = (JackrabbitRepository)repositoryFactory.getRepository(prop);
+ } catch (RepositoryException e) {
+ failWithException(e);
+ };
+ }
+
+ private void login() {
+ try {
+ sessionGarbageCollector = repository.login(new SimpleCredentials("admin", "admin".toCharArray()));
+ sessionMover = repository.login(new SimpleCredentials("admin", "admin".toCharArray()));
+ } catch (Exception e) {
+ failWithException(e);
+ }
+ }
+
+ private GarbageCollector setupGarbageCollector() {
+ try {
+ return ((SessionImpl) sessionGarbageCollector).createDataStoreGarbageCollector();
+ } catch (RepositoryException e) {
+ failWithException(e);
+ }
+ return null;
+ }
+
+ private void failWithException(Exception e) {
+ fail("Not expected: " + e.getMessage());
+ }
+
+ private int getBinaryCount(GarbageCollector garbageCollector) {
+ int count = 0;
+ Iterator<DataIdentifier> it;
+ try {
+ it = garbageCollector.getDataStore().getAllIdentifiers();
+ while (it.hasNext()) {
+ it.next();
+ count++;
+ }
+ } catch (DataStoreException e) {
+ failWithException(e);
+ }
+ log("Binary count: " + count);
+ return count;
+ }
+
+ private void log(String message) {
+ logger.debug(message);
+ //System.out.println(message);
+ }
+}
Modified: jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java?rev=1367435&r1=1367434&r2=1367435&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java Tue Jul 31 07:07:18 2012
@@ -51,6 +51,7 @@ public class TestAll extends TestCase {
suite.addTestSuite(TempFileInputStreamTest.class);
suite.addTestSuite(TestTwoGetStreams.class);
suite.addTestSuite(WriteWhileReadingTest.class);
+ suite.addTestSuite(GCSubtreeMoveTest.class);
return suite;
}