Index: src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java (revision 0) +++ src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java (working copy) @@ -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 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); + } +} Property changes on: src/test/java/org/apache/jackrabbit/core/data/GCSubtreeMoveTest.java ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Index: src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java (revision 1367073) +++ src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java (working copy) @@ -97,6 +97,16 @@ 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 @@ 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 @@ } /** + * 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 { Index: src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java (revision 1367073) +++ src/main/java/org/apache/jackrabbit/core/data/GarbageCollector.java (working copy) @@ -18,6 +18,7 @@ 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 @@ 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 @@ 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 GarbageCollector( DataStore dataStore, IterablePersistenceManager[] list, - Session[] sessionList) { + SessionImpl[] sessionList) { this.store = dataStore; this.pmList = list; this.persistenceManagerScan = list != null; @@ -162,7 +165,7 @@ } if (pmList == null || !persistenceManagerScan) { - for (Session s : sessionList) { + for (SessionImpl s : sessionList) { scanNodes(s); } } else { @@ -174,11 +177,11 @@ } } - 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 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 @@ } catch (InvalidItemStateException e) { LOG.debug("Node removed concurrently - ignoring", e); } + checkObservationException(); } private void rememberNode(String path) { @@ -368,6 +369,24 @@ } } + 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 @@ */ 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 @@ // ignore } } catch (Exception e) { - lastException = e; + gc.onObservationException(e); + try { + stop(); + } catch (RepositoryException e2) { + LOG.warn("Exception removing the observation listener - ignored", e2); + } } } } Index: src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java (revision 1367073) +++ src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java (working copy) @@ -1391,7 +1391,7 @@ 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); Index: src/test/java/org/apache/jackrabbit/core/data/TestAll.java =================================================================== --- src/test/java/org/apache/jackrabbit/core/data/TestAll.java (revision 1367073) +++ src/test/java/org/apache/jackrabbit/core/data/TestAll.java (working copy) @@ -51,6 +51,7 @@ suite.addTestSuite(TempFileInputStreamTest.class); suite.addTestSuite(TestTwoGetStreams.class); suite.addTestSuite(WriteWhileReadingTest.class); + suite.addTestSuite(GCSubtreeMoveTest.class); return suite; }